Noticed this while looking at bug 862249 (the benchTypedArraySet test there). Accessing a typed array property like byteLength is super slow: -- function f(arr) { var res = 0; for (var i=0; i<1000000; i++) { res = arr.byteLength; } return res; } var t = new Date; f(new Int32Array(1000)); print(new Date - t); -- d8 : 16 ms js no-ion : 171 ms js : 226 ms Note that Ion knows how to optimize .length and indeed that's very fast: js : 1 ms d8 : 16 ms I think we should: (1) Make the getter IC work here; it's probably failing because typed arrays are non-native. (2) Have Ion inline .byteOffset/.byteLength/etc like .length. These are just int32 values stored in fixed slots; should be straight-forward.

Note: I know LICM is hoisting the .length access out of the loop. But that's the point: inlining .byteLength etc allows us to do the same for these other properties.

This explains a lot! I never considered the possibility that only certain properties got 'special' optimization treatment. Is there a list of them anywhere?

(In reply to Kevin Gadd (:kael) from comment #2) > This explains a lot! I never considered the possibility that only certain > properties got 'special' optimization treatment. Is there a list of them > anywhere? We inline .length on arrays, typed arrays, strings, arguments.. Fixing the getter IC to work with typed arrays brings this down to 12 ms, 4 ms faster than v8. I also want to do (2) in comment 0 but (1) should fix the worst problems.

Fixes the Baseline ICs to cache properties on typed arrays, and fixes IonBuilder so that our inlined-getter-call code works for getters (like byteLength) on typed arrays. Ion should really inline these properties directly without calling the getter (they just load a value from a fixed slot), but for now this is much better than what we were doing. For the micro-benchmark in comment 0: before: 220 ms after: 10 ms (We're not hoisting the property access, once we optimize this property in Ion we can do that.)

Comment on attachment 8333772 [details] [diff] [review] Patch Review of attachment 8333772 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jit/IonBuilder.cpp @@ +5932,5 @@ > +{ > + if (IsTypedArrayClass(clasp)) { > + // Typed arrays have a lookupGeneric hook, but it only handles > + // integers, not names. > + JS_ASSERT(name); Maybe move this assert above the condition?

https://hg.mozilla.org/integration/mozilla-inbound/rev/5ef1eb4f5d57 (In reply to Brian Hackett (:bhackett) from comment #5) > Maybe move this assert above the condition? I think it's better to keep it below the comment, to make it clear why the assert is there.

(In reply to Jan de Mooij [:jandem] from comment #6) > I think it's better to keep it below the comment, to make it clear why the > assert is there. Yeah, though when I see asserts inside a conditional I generally figure that assert might not hold in the other branch, whereas in this case name should always be non-NULL. I guess both the assert and comment could be hoisted.