| 20:05 | <snek> | what's the perf of Object.hasOwn vs Object.prototype.hasOwnProperty.call? (re https://github.com/nodejs/node/pull/54710#issuecomment-2325374488 ) |
| 20:05 | <snek> | it could be improved though |
| 20:14 | <ljharb> | i'm sure it's accurate. but it doesn't make any sense to me why it'd be implemented in that way |
| 20:14 | <shu> | how would you implement it? |
| 20:14 | <snek> | i would imagine it was convenient at the time. generally v8 development goes like 1) implement thing correctly and then 2) optimize given the known working impl |
| 21:42 | <canadahonk> | fwiw I implemented it basically the same in my engine |
| 23:04 | <ljharb> | i would assume that whatever hasOwnProperty does to check that the object has the property is the same thing hasOwn would do |
| 23:04 | <ljharb> | as opposed to just wrapping hasOwnProperty |
| 23:27 | <bakkot> | then all of the various optimizations which look for hasOwnProperty would have to be updated to look for both hasOwn and hasOwnProperty |
| 23:33 | <shu> | i would assume that whatever hasOwnProperty does to check that the object has the property is the same thing hasOwn would do |
| 23:42 | <ljharb> | it wouldn't, it'd make it "no slower" |
| 23:43 | <shu> | best to benchmark the current difference, i expect a few branches won't matter too much in practice if you think hasOwn is better stylistically |
| 23:44 | <Justin Ridgewell> | I don’t think the call is the slow part of the implementation? |
| 23:44 | <Justin Ridgewell> | It’s the extra checks done before the call |
| 23:46 | <ljharb> | node folks get very perf-sensitive |
| 23:46 | <ljharb> | so even the mere suggestion that hasOwn is slower caused the entire PR i linked to be abandoned. |
| 23:47 | <shu> | It’s the extra checks done before the call |
| 23:48 | <Justin Ridgewell> | Definitely not expensive, but they’re extra work vs not doing the checks. |
| 23:48 | <shu> | well of course, the question is does it matter |
| 23:48 | <bakkot> | https://source.chromium.org/chromium/chromium/src/+/main:v8/src/builtins/builtins-object-gen.cc;l=690-717 (oh I see it was already linked in the node thread, oops) |
| 23:48 | <Meghan Denny> | fwiw mdn is recommending hasOwn over hasOwnProperty |
| 23:48 | <Meghan Denny> | https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/hasOwn |
| 23:49 | <shu> | if the goal is to use 'hasOwn' for whatever reason, and that reason needs to be traded off against possible performance regressions, either you are willing to quantify the regression or not |
| 23:49 | <shu> | if you are not willing to quantify the regression (and find out if it matters), that says to me the "whatever reason" for wanting to use hasOwn is already pretty weak |
| 23:50 | <shu> | and the thing you should do is to convince node maintainers that it's worth the price of finding out if it's slower in a way that matters, rather than VM optimizations |
| 23:50 | <Justin Ridgewell> | well of course, the question is does it matter hasOwnProperty.call(obj, key), but node already has a devirtualized helper. |
| 23:50 | <shu> | right, that makes sense to me |
| 23:50 | <ljharb> | the issue is that that helper already has to do a bind |
| 23:51 | <ljharb> | so they were hoping to drop the bind/call/apply. but v8 does that internally for hasOwn, so it's probably a wash |
| 23:51 | <shu> | a .bind is very different than a .call/apply |
| 23:51 | <shu> | a .bind does an allocation, v8 does not internally do that? |
| 23:51 | <shu> | it just calls the builtin directly |
| 23:52 | <ljharb> | 🤔 |
| 23:53 | <Justin Ridgewell> | fwiw mdn is recommending hasOwn over hasOwnProperty Object.hasOwn is a definite improvement and they shouldn’t worry about any performance change. |
| 23:53 | <shu> | like, the IsNullOrUndefined check is pretty cheap |
| 23:54 | <shu> | most of the cost i imagine is the actual builtin call |
| 23:54 | <shu> | but this is all moot -- if the reason is that it doesn't improve the code anyway |