Compared Dromaeo between nightly Interp+JM+Ion and BL+Ion, and got the following results: http://dromaeo.com/?id=192697,192702 (BL+Ion on left hand side, Interp+JM+Ion on right hand size) We're on par on most tests, but worse on slightly under half of them. A good chunk of this is probably caused by JSNative/PropertyOp handlers for getters and setters, as well as not handling ListBase proxies in baseline ICs.

Taking a look at implementing this, we're already duplicating a bunch of extremely similar functions across IonCaches.cpp, and BaselineIC.cpp. Implementing this would add to that, with a new |IsCacheableListBase| method along with a bunch of other ones that are now duplicated between the two files. If we don't rectify this now it'll just become harder and harder to factor together as the logic for checking optimizability subtly deviates between the two engines. Unless there's a really good reason, that logic should be shared between Ion and Baseline. Before doing this I'm going to land some refactors on inbound that allows Baseline to share the procedures that Ion uses for checking for optimizability, as well some helper codegeneration procedures that should be shareable.

This moves a bunch of static helper functions defined in IonCaches.cpp, and moves them into a separate file, signatures exported via a separate header.

Please coordinate with peterv. He has a bunch of outstanding changes to the listbase stuff that you're probably bitrotting here....

Comment on attachment 731920 [details] [diff] [review] Refactor IonCaches checker and helper logic into separate file. Review of attachment 731920 [details] [diff] [review]: ----------------------------------------------------------------- Doing a diff between the old file and the new file does not show any unexpected modification of the functions, except for the expected removal of static keywords. Maybe we can find a better name for this file, such as IonCacheHelpers ? Note: This move the definitions of these static functions from the global namespace to the namespace js::ion, which is fine.

Holding off on the refactoring for now until 1) we figure out a better name for the VMObjectJIT file, and 2) Baseline lands. In the meantime, ran perf tests with the patch for the sub bug 856829. On linux, 32-bit, BL (left) vs. Trunk (right): http://dromaeo.com/?id=192848,192854 Now we're at par or winning on all but 5 benchmarks, overall. The losses are entirely on Prototype benchmarks, and a good chunk of that, if not all of it, can probably be accounted for by ListBase optimizations.

> The losses are entirely on Prototype benchmarks And as discussed on IRC jQuery's trigger().

Ran pre-BC and post-BC firefox on OSX, and there are more significant regressions than what showed up on 32-bit Linux builds: http://dromaeo.com/?id=193002,193003

Ah, that more closely matches what we saw on Talos.... I wonder whether it's 32-bit vs 64-bit somehow? Though Talos had issues for other builds too, iirc.

I tried both, and the regressions were comparable. I just posted the 64-bit, but here are the 32-bit scores on OSX to round it out: http://dromaeo.com/?id=193005,193006 I'm working on getting ListBase stuff into Baseline ICs and seeing how that changes things. The big difference in DOM attributes/element.property suggests an issue there.

So the element.property thing.... if you notice one of those numbers has +/-100% error bars. ;) That test is now basically testing a no-op loop once we ion-compile it. So if anything weird (gc?) happen anywhere in there the numbers will be all over the place.

(In reply to Boris Zbarsky (:bz) from comment #10) > So the element.property thing.... if you notice one of those numbers has > +/-100% error bars. ;) > > That test is now basically testing a no-op loop once we ion-compile it. So > if anything weird (gc?) happen anywhere in there the numbers will be all > over the place. That test should be fixed then :) If Ion is DCEing it, then it's basically not doing its job in testing dom performance anymore. Anyway, in other news I finished adding a ListBase GetProp/CallNative stub (that's the only one I could see being hit, so I didn't bother to implement CallScripted or ReadSlot variants). Numbers improved. The following graph is (BL+ListBaseProxy,BL plain,pre-BL) on 32-bit Linux: http://dromaeo.com/?id=193285,192848,192854 Some nice improvements, but nothing earth shattering. About a 10-15% improvement across the Prototype benches that were slowing down before, but still far short of previous scores (but this was expected, arguments support is the key for Prototype, which I'll start working on now). I sent off a try build to get OSX binaries and I'll put numbers for those up when I get them.

> That test should be fixed then :) Sure, but I'm not going to worry about it. ;) > Anyway, in other news I finished adding a ListBase GetProp/CallNative stub (that's the > only one I could see being hit, so I didn't bother to implement CallScripted or ReadSlot > variants). Hmm. If a page adds things to HTMLCollection.prototype, those last could be hit, no? But I agree they're lower priority.

Nominating for tracking-firefox23 since this is a performance regression from the baseline compiler landing.

Just posted the Ion arguments-object support patch to bug 860145. No direct perf gains from this change, but it sets the stage for further optimizations. I'd like feedback on whether the following exposition makes sense: But first, an explanation: As I noted in the bug, a microbench targeted at the patch shows about a 16% gain. The main conclusion I can come to is that the combined cost of the C++ call to create the arguments object, and slowpath access of |arguments.length| and |arguments[i]| (which become object reads), ends up eating heavily into the perf gains reaped by ion-compiling the script. 16% gain going from Baseline => Ion is pretty sad. However, I think there is a reasonable way in which these overheads can be eliminated. First, here's a listing of all references to arguments in prototype.js: http://pastebin.mozilla.org/2300920 These classify into the following forms: arguments.length arguments[i] (GETELEM) fn.apply(this, arguments) slice.call(arguments) AND slice.call(arguments, i) [].concat(arguments) $A(arguments) update(someArray, arguments) merge(someArray, arguments) The first 4 are easily identifiable as read-only patterns, and their corresponding ops will always have MCreateArgumentsObject as a direct input, making them easy to pattern-match. The last 3 cases, where arguments is passed to the call, are stable calls whose bodies will be inlined (thus the top-level MCreateArgumentsObject will be used within them directly), and they themselves only perform operations the usual GETPROP/LENGTH/etc. operations on the object. So this leads to a situation where there is an easily pattern-matchable set of ops, which show up idiomatically (note that jquery's usage of slice.call(arguments) is captured here), which are all read-only accesses on arguments. This suggests the utility of a quick optimization pass with the following properties: 1. Only runs if |script->needsArgsObj()| 2. Checks all users of MCreateArgumentsObject 3. If all users are one of the following: * MGetArgumentsObjectArg or MSetArgumentsObjectArg (getarg/setarg). * MArgumentsObjectLength(arguments) * MGetArgumentsObjectElem(arguments, int-typed-value-or-constant) * MFunApplyArgumentsObject(blah, arguments) * MSliceArgumentsObject(arguments, maybe?<int-typed-value>) * MArrayConcatArgumentsObject(someArray, arguments) 4. Remove and replace the MCreateArgumentsObject with an MConstant undefined. 5. Replace the given instructions with specialized variants that use the MParam slots directly, or read from the arguments values stored in the Ion frame. Some extra notes: * If JSOP_SETARG is present in the JSScript, the optimization of these operations will be slightly compromised. What I want to do is have JSOP_SETARG directly modify the IonFrame on the C stack and overwrite the incoming argval. Not sure if that'll require a write barrier or not (since it's a stack value being overwritten). Moreover, all the other optimizable arguments-related ops will need to be changed to use the IonFrame on stack to do their thing (e.g. JSOP_GETARG will be a stack read). * If JSOP_SETARG is NOT present in the JSScript, the optimization can be better. We can say deterministically that args aliasing doesn't matter (since arguments object is never modified), and go ahead and make replace the MIR instructions for the ops with variants that directly use the MParam slots. The latter variant (JSOP_SETARG not being present), is actually the most prevalent case in prototype. The one case where it is present is in the "delay" method. However, jquery's usage of arguments-heavy scripts does contain JSOP_SETARGS, in particular it contains it in the jquery.trigger control flow that we regress with, so the former variant can't really be ignored.

Additional observations: |fn.apply(blah, arguments)| actually doesn't show up in |needsArgsObj| functions all that much. It shows up in jquery in a small helper method called 'slice', and nowhere in Prototype. Should be able to skip handling this in heavy-args scripts and still post the same perf gains. Similarly, |[].concat(arguments)| shows up in one place in Prototype, and nowhere in jquery. Also, the one place it shows up in Prototype is in a predicate method called "CONCAT_ARGUMENTS_BUGGY". I suspect it's not that important to handle ;) So we should be ok checking just for the cases: arguments.length arguments[i] (GETELEM) slice.call(arguments) AND slice.call(arguments, i)

Ok my cunning plan has a problem. We forbid inlining of functions with loops or funcalls within them. Sigh. We really need to enable inlining of functions with loops...

Did some profiling today. Running with the Ion-argsobj support patch, profiler shows that in DOM Attributes (Prototype) and DOM Events (Prototype), about 35% of the time is spent in the update() method, which is called with |arguments| from wrapped methods generated by |methodize|. Seems to confirm suspicion that arguments-object handling is a culprit here.

I think there's a lot of overlap between your plan in comment 14 and the intended purpose of the arguments optimization analysis we already do MagicValue(JS_LAZY_ARGUMENTS). It would be nice if there was only one place we did this sort of thing, and Ion compilation seems the better place. The existing analysis is also a consumer of the analysis info which we are trying to get rid of. Taking further discussion to bug 861596 though...

Clearning dvander needinfo. Alternative approaches have proven useful already in this. No need to talk about super fancy transforms ATM.

Without an engineering driver here and no specific target to hit on "improve performance" there's no reason for us to track this.

> and no specific target to hit on "improve performance" There's a specific target: "parity with Firefox 22".

On that point: OSX 64-bit Dromaeo Runs on FF22 vs. Nightly: FF22: http://dromaeo.com/?id=198532 Nightly: http://dromaeo.com/?id=198533 Comparison: http://dromaeo.com/?id=198532,198533 jQuery.trigger is still down heavily. And DOM Traversal (Prototype & jQuery) are down overall.

That said, unless we plan to actually change something for 23 (and now 24?) here, it doesn't seem like tracking is worthwhile. :(

Those tests are using versions of jQuery that were released 2 years ago, so I've updated Dromaeo source to the latest 1.10.2 release and duplicated the same tests for jQuery 2.x. https://github.com/jeresig/dromaeo/commit/124a0f8e0b115798d775c609b444f45add39cca3

> so I've updated Dromaeo source to the latest 1.10.2 release Now if we can get the new tests pushed out to dromaeo.com and our internal test suite...

I am revisiting this issue, trying to see what the problems are. I re-ran dromaeo on Linux 32-bit and OSX 64-bit with the latest nightly. Linux 32-bit: http://dromaeo.com/?id=201540,192854 OSX 64-bit: http://dromaeo.com/?id=201529,193003 Linux 32-bit regressions: DOM Events (jQuery) trigger - 0.76 to 0.13 DOM Query (Dojo) div.character, div.dialog - 3916 to 3752 DOM Query (jQuery) body - 312893 to 157970 DOM Traversal (Prototype) all of them regress slightly DOM Traversal (jQuery) parents x 10 - 36.84 to 31.75 prev x 10 - 153.78 to 134.15 next x 10 - 139.77 to 128.02 OSX 64-bit regressions: Many individual regressions. Too many to list. First order of business is jQuery.trigger - highly regressed on both 32 and 64.

I profiled the jQuery.trigger regression, and it led to a particular method (jQuery.event.trigger) which was not being ion-compiled because it had a try/catch block in it. Jan recently checked in support for ion-compiling try/catch (but preffed off). Preffing it on and re-running on linux produced these scores (tip + try/catch preffed on VS pre-baseline-landing scores). http://dromaeo.com/?id=201853,192854 The score for jQuery.trigger gets better (from 0.9 to 0.13, but not that much). Profiling shows that majority of time is still spent in this function. We may be bailing or invalidating more, perhaps. Weirdly, running with try/catch ion-compilation enabled seems to regress a bunch of other benchmarks compared to tip. Once again, we may be invalidating or bailing more with try/catch compilation on: http://dromaeo.com/?id=201853,201540 Looking into this right now.

Ah, I shouldn't be comparing 201853 to 201540. The former is my linux build and the latter is a nightly build. Just disabled try/catch compilation and re-ran the scores with my own build and it seems like try/catch compilation is a strict win: (Dromaeo: with ion try/catch compilation vs without ion try/catch compilation) http://dromaeo.com/?id=201853,201855 However, this does not have any effect on jQuery.trigger. I don't think the hot function (jQuery.event.trigger()) gets called enough times to get selected for ion-compilation anyway. Running the page directly indicates that the function gets called 200 times. In the jaeger days, it would have gotten compiled at useCount=40 and 160 of those runs would have been in jaeger jitcode. Now it runs entirely in baseline, which is slower than jaeger. I'm going to run some other basic tests on the code to ensure there isn't any obvious gains to be made in optimized IC stubs. If that doesn't yield anything, the best bet seems to be looking into introducing a "cheap ion compile" phase.

It seems that there are some existing issues to solve before going down the path described above. I enabled baseline spew, ran jQuery.trigger in the browser, and counted the number of fallback hits that happened. I found the following (number at start of line is count of fallback hits): 1022 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:1444) (pc=674,line=1522,,): Compare(stricteq) 1026 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=39,line=2595,,): Compare(stricteq) 1026 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=85,line=2604,,): GetProp(getprop) 1027 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=404,line=2653,,): NewObject 1027 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=558,line=2662,,): NewArray 1027 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=575,line=2663,,): SetProp(setprop) 1027 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=596,line=2667,,): GetProp(getprop) 1027 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=657,line=2673,,): GetElem(getelem) 1027 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=664,line=2673,,): NewObject 1027 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=862,line=2693,,): GetProp(getprop) 1027 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=983,line=2705,,): SetElem(setelem) 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2590) (pc=1001,line=2650,,): GetElem(getelem) 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=265,line=2865,,): GetElem(getelem) 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=39,line=2849,,): NewArray 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=527,line=2881,,): SetProp(setprop) 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=791,line=2916,,): Compare(ne) 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=832,line=2916,,): NewArray 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=9,line=2848,,): GetProp(getprop) 1029 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1331,line=2945,,): GetElem(getelem) 1029 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1338,line=2945,,): NewObject 1029 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1351,line=2947,,): GetProp(getprop) 1029 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1530,line=2956,,): GetElem(getelem) 1090 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:630) (pc=358,line=660,,): Compare(stricteq) 1578 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1036,line=2929,,): Call(funapply) 1579 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2978) (pc=206,line=2986,,): SetElem(setelem) 1886 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:1650) (pc=46,line=1652,,): GetElem(getelem) 2058 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1185,line=2939,,): GetProp(getprop) 2061 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1171,line=2939,,): GetProp(getprop) 2551 [BaselineIC] Updating existing stub with generation, old value: 1, new value: 1 2556 [BaselineIC] Generating GetProp(DOMProxyObjWithGeneration/NativeGetter 0xf28f8702) stub 5694 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1085,line=2933,,): GetElem(getelem) 15791 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2978) (pc=362,line=3001,,): Call(funapply) The highest count (last line) event is a funapply call that baseline doesn't optimize because the argument is an array (instead of lazy arguments). It happens because jquery.event.handle (jquery.js:2978) doesn't get ion-compiled (because of TI interaction with the lazy-arguments type), and it runs in baseline, and the JSOP_FUNAPPLY gets run entirely in baseline. Ion needs to be fixed so it compiles that function, but that requires some modification to snapshot code, according to nbp. It's a bit more involved, and the listing above shows other, easier to address issues in baseline itself, so I'll get to those first. Note the large number of fallback hits happening for jquery.js:2846 (jquery.event.trigger): 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=265,line=2865,,): GetElem(getelem) 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=39,line=2849,,): NewArray 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=527,line=2881,,): SetProp(setprop) 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=791,line=2916,,): Compare(ne) 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=832,line=2916,,): NewArray 1028 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=9,line=2848,,): GetProp(getprop) 1029 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1331,line=2945,,): GetElem(getelem) 1029 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1338,line=2945,,): NewObject 1029 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1351,line=2947,,): GetProp(getprop) 1029 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1530,line=2956,,): GetElem(getelem) 1578 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1036,line=2929,,): Call(funapply) 2058 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1185,line=2939,,): GetProp(getprop) 2061 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1171,line=2939,,): GetProp(getprop) 5694 [BaselineICFallback] Fallback hit for (file:///home/kvijayan/Scratch/jslib-event-jquery_files/jquery.js:2846) (pc=1085,line=2933,,): GetElem(getelem) There are, in total, about 21000 fallback hits for various GETPROPS, GETELEMS, and FUNAPPLY operations. These can be tracked down and stubs added for them in baseline. I'll start with funapply since it'll help speed up |jquery.event.handle| as well. Then I'll get around to fixing up the GETPROPS and GETELEMS, and then look into fixing the TI issue that prevents |jquery.event.handle| from getting ion-compiled.

Measured again after the GETELEM and FUNAPPLY patches landed: http://dromaeo.com/?id=204957,192854 We're doing much better on DOM Events (jQuery). The overall benchmark is about 40% faster than pre-baseline scores, with jQuery.trigger improved roughly 6-fold, now standing at 30% slower than the pre-baseline score. Looking into DOM Traversal next.

Very much fixed, Kannan?

Yeah, closing this after consulting with bz. we'll open a new bug when we look at dromaeo perf again.