1225311 - Simple Float32Array stores are not being optimized

Status: RESOLVED DUPLICATE of bug 1225821

(Core :: JavaScript Engine: JIT, defect)

Description

[Benoit Girard (:BenWa)]

Attachment: slow_typed_array.html

http://people.mozilla.org/~bgirard/cleopatra/#report=5168147ced674791e67384bb723885db069bc36a&selection=0,1950,1951,1952,1953,1807,1954,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,763,764,765,1955,1956,1957,1958,625,29,30,36,38,222,1959,1960,1961,256

I had a demo that was creating Float32Array by first creating a JSArray, pushing millions of values then constructing a Float32Array using the JSArray constructor. When 'optimized' my demo to instead allocated the Float32Array first and then do stores into the array the performance got significantly worse.

Looking at the profiles it looks like the stores are not being optimized properly. I see a lot of samples in:
js::SetObjectElement(JSContext*, JS::Handle<JSObject*>, JS::Handle<JS::Value>, JS::Handle<JS::Value>, bool)

where I would expect a more optimal store method.

Comment 1

[Lars T Hansen [:lth]]

Weird.  In the shell the generated machine code for this is very nice, and even with off-thread compilation most loop iterations run in jitted code, according to the -D profile. (x86_64, Linux, current m-i.)

Comment 2

[Benjamin Bouvier [:bbouvier] (inactive)]

At start, MStoreUnboxedScalar is used, which is the most optimal way of storing to a known typed array, but after some recompilation, MCallSetElement is used (which might be the caller of js::SetObjectElement), which is slower.

Before this recompilation, the array elements is known and has type MIRType_Elements. After this recompilation, it has the generic MIRType_Value, so we somehow lose information. Why?

After some investigation, this happens because the typeset associated to the array becomes an unknown object, causing jit::ElementAccessIsAnyTypedArray to return false (types->getTypedArrayType(constraints) returns Scalar::MaxTypedArrayView), and IonBuilder::setElemTryTypedArray to not emit the optimal path.

Why do we lose the typeset is the next question...

Comment 3

[Jan de Mooij [:jandem]]

Based on comment 2 and glancing at the testcase, it could be the same issue as bug 1205427 comment 1.

It's still on my TODO list to fix that, maybe I should prioritize it for this week because it's showing up on real-world code.

Comment 4

[Benjamin Bouvier [:bbouvier] (inactive)]

Now that bug 1225821 has landed, I see that only MStoreUnboxedScalar being generated. Benoit, is it fixed for you? The fix should be in one of the next nightlies.

Comment 5

[Benoit Girard (:BenWa)]

Yea looks good!