Going through js::Call and obj_toString slows us down on Ember. It seems we can short-circuit this in OrdinaryToPrimitive. André, any thoughts on this? I wrote the patch like this to not make things much slower when objects override toString.
(In reply to Jan de Mooij [:jandem] from comment #0) > Going through js::Call and obj_toString slows us down on Ember. It seems we > can short-circuit this in OrdinaryToPrimitive. Do you already have any numbers how much this will improve Ember resp. the part of Ember which repeatedly calls obj_toString? > André, any thoughts on this? I wrote the patch like this to not make things > much slower when objects override toString. Hmm, on the one hand I kind of don't like to have an Ember-specific optimisation in OrdinaryToPrimitive. But on the other hand a single run of Speedometer/Ember calls 1.7M times obj_toString (bug 1383795, comment #1). So then I measured how often OrdinaryToPrimitive (with hint == JSTYPE_STRING) is called for each framework in Speedometer: # calls here  # your optimisation applies Vanilla 7 0 Vanilla2015 22 0 Vanilla-babel 0 0 React 0 0 React-Redux 0 0 Ember 1692181 1692173 Backbone 204 200 AngularJS 202 0 Angular2 2 0 Vue 0 0 jQuery 685 0 Preact 0 0 Inferno 0 0 Elm 0 0 Flight 206 0 And based on these numbers, at least for the frameworks tested in Speedometer, all frameworks except Ember have a negligible number of calls to OrdinaryToPrimitive, so having this specific optimisation shouldn't slow down other parts of Speedometer. I'm still a bit undecided on whether or not I'd go with this change, but if we apply the patch, we should probably also file a follow-up bug to investigate a more general solution for this problem. (So we can look into this issue in a year or so. ;-) Given that I'm still undecided, I'd be great to have a second opinion. Maybe ask :djvj, too, since he's currently also looking into built-in optimisations?  http://searchfox.org/mozilla-central/rev/ad093e98f42338effe2e2513e26c3a311dd96422/js/src/jsobj.cpp#3075
(In reply to André Bargull from comment #1) > Do you already have any numbers how much this will improve Ember resp. the > part of Ember which repeatedly calls obj_toString? It's a measurable improvement on Ember. I don't remember how much exactly but more than a few %, I can get more data on this. I don't think it's that Ember-specific, it also hits on non-Ember-using websites. I'll do some measurements of that too :) FWIW, Chakra has a similar optimization where they check for Object.prototype.valueOf/toString: https://github.com/Microsoft/ChakraCore/blob/e063e2a6527389c54f3b2dd181f9669c7140fbe4/lib/Runtime/Types/DynamicType.cpp#L395-L418
Do we still want to look into this optimization? It's been over a week and no one stepped up to veto this approach, so maybe just go ahead and try it out on Speedometer (and other benchmarks), and if it shows improvements (and there are no regressions), we'll take it. WDYT?
Comment on attachment 8890346 [details] [diff] [review] Patch Clearing f? for now per comment #3.
Assignee: jdemooij → nobody
Status: ASSIGNED → NEW
You need to log in before you can comment on or make changes to this bug.