Closed Bug 1142843 Opened 5 years ago Closed 5 years ago

Poor performance of Typed Objects

Categories

(Core :: JavaScript Engine, defect)

x86_64
Linux
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla39
Tracking Status
firefox39 --- fixed

People

(Reporter: arno, Assigned: bhackett)

Details

Attachments

(1 file)

We have done some benchmarks on Nightly and found a significant performance
impact when using Typed Objects. It was our understanding that Typed Object
introduce immutable types that can be handled more efficiently in the JS
engine, so we are a bit at a loss why they introduce a higher overhead.

Here are the sources we used for the test (both the plain JS version as
well as the version using Typed Objects):
https://github.com/wangyanxing/TSBenchmark/tree/gh-pages/benchmark/vec3_norm

Here are clickable pages you can load in Nightly:

The JS version:
http://wangyanxing.github.io/TSBenchmark/benchmark/vec3_norm/class.html

The Typed Object version:
http://wangyanxing.github.io/TSBenchmark/benchmark/vec3_norm/struct.html
Flags: needinfo?(bhackett1024)
> that can be handled more efficiently in the JS engine

Can be, once you add the special fast paths for them...  which might not be there yet across the board.
Yeah, the problem here is that we are missing an optimized path for the v.length() call, and end up calling into the VM repeatedly rather than statically knowing the function being called.  If I "fix" this by making length() a function on the global rather than the Vector prototype, the typed objects version becomes slightly faster, 116ms vs. 120ms after scaling up the test time 10x.  I suspect the difference being so small is because our optimization of typed object initialization isn't great, and we need to write the initial zero values of the typed object fields before filling them in with the final value.  If I turn on unboxed objects (a WIP feature that uses a layout very similar to typed objects for well behaved normal objects), which don't have this initialization issue, I get a time of 103ms.

I'll write a patch to fix the .length() calling issue, which is the main problem here.  It might be a while before the initialization thing gets fixed (we need to have a better sense of how typed objects get initialized in practice), though that's a minor optimization in comparison and typed objects should still be faster (especially in larger, non-trivial programs) than normal objects.
Attached patch patchSplinter Review
Fix baseline and ion caches, and ion constant inlining to handle accesses on a prototype where the receiver is a typed object.  This fixes things so that the unmodified testcase behaves as in comment 2.

Looking at the testcase some more, there is another deficiency which is I think more important than extra writes during initialization.  When reading double values out of an object we need to check them and canonicalize any NaN values read out.  This canonicalization is needed for security, as we represent e.g. object values using special NaN values and if the program has another view on the typed object's buffer then it could forge NaNs that would be interpreted as objects when read out later.  This forging is only possible if other views have been created on the object's buffer, which will almost never be the case.  I'll try to write a patch to improve this behavior within the next week or two, this has been kind of a longstanding problem which affects typed arrays as well.
Assignee: nobody → bhackett1024
Flags: needinfo?(bhackett1024)
Attachment #8577788 - Flags: review?(jdemooij)
(In reply to Brian Hackett (:bhackett) from comment #3)
> Created attachment 8577788 [details] [diff] [review]
> patch
> 
> Looking at the testcase some more, there is another deficiency which is I
> think more important than extra writes during initialization.  When reading
> double values out of an object we need to check them and canonicalize any
> NaN values read out.  This canonicalization is needed for security, as we
> represent e.g. object values using special NaN values and if the program has
> another view on the typed object's buffer then it could forge NaNs that
> would be interpreted as objects when read out later.  This forging is only
> possible if other views have been created on the object's buffer, which will
> almost never be the case.

It should never be possible to read out a user-provided bit pattern as a Value. IIUC, any typed object that can store pointers is opaque, so you can't get another view of its buffer.

Typed array NaN canonicalization is per spec, at least in a user-visible way. https://people.mozilla.org/~jorendorff/es6-draft.html#sec-getvaluefrombuffer step 9b for reading, the next section for writing. I assumed that typed objects would do the same?
(In reply to Steve Fink [:sfink, :s:] from comment #4)
> It should never be possible to read out a user-provided bit pattern as a
> Value. IIUC, any typed object that can store pointers is opaque, so you
> can't get another view of its buffer.

These typed objects aren't opaque, they just have three doubles in them.

> Typed array NaN canonicalization is per spec, at least in a user-visible
> way.
> https://people.mozilla.org/~jorendorff/es6-draft.html#sec-getvaluefrombuffer
> step 9b for reading, the next section for writing. I assumed that typed
> objects would do the same?

Presumably, yes.  Just because NaN canonicalization is part of the spec doesn't mean we always need to actually *do* the canonicalization when loading double values out of typed objects or arrays.  In this case, we can try to prove that the canonicalization is not necessary since any NaN read is already canonical.  Doing this would require knowing that the only view on the data is a double, and that any NaNs written to those doubles are canonical.  The first part follows from knowing there aren't any other views on the data besides the typed object or typed array, and the second part would need to follow from the behavior of the VM but is a property that spec seems to require as well.
Hi all,

I rebuilt Nightly with Brian's patch, there is an obvious improvement indeed!
However I tried my previous benchmark and TypedObject is still slower than JS object.

JS version:
http://wangyanxing.github.io/TSBenchmark/benchmark/class.html
Source:
https://github.com/wangyanxing/TSBenchmark/blob/gh-pages/benchmark/src.js

Typed Object version:
http://wangyanxing.github.io/TSBenchmark/benchmark/struct.html
Source:
https://github.com/wangyanxing/TSBenchmark/blob/gh-pages/benchmark/src_opt.js


May I have some hints from you guys? Thanks a lot!!
Attachment #8577788 - Flags: review?(jdemooij) → review+
(In reply to Yanxing Wang from comment #6)
> Hi all,
> 
> I rebuilt Nightly with Brian's patch, there is an obvious improvement indeed!
> However I tried my previous benchmark and TypedObject is still slower than
> JS object.
> 
> JS version:
> http://wangyanxing.github.io/TSBenchmark/benchmark/class.html
> Source:
> https://github.com/wangyanxing/TSBenchmark/blob/gh-pages/benchmark/src.js
> 
> Typed Object version:
> http://wangyanxing.github.io/TSBenchmark/benchmark/struct.html
> Source:
> https://github.com/wangyanxing/TSBenchmark/blob/gh-pages/benchmark/src_opt.js
> 
> 
> May I have some hints from you guys? Thanks a lot!!

What times are you seeing, and what platform are you on?  With the patch now in the tree, I get times of 10-20% faster in the typed objects version when running in the shell (i.e. after stripping out uses of document and canvas) on x86 and x64.
Thanks for the reply!

I did the test early last week and my platform is Windows 8.1 x64.

I'll try to remove the uses of document/canvas and then try again!


(In reply to Brian Hackett (:bhackett) from comment #8)
> (In reply to Yanxing Wang from comment #6)
> > Hi all,
> > 
> > I rebuilt Nightly with Brian's patch, there is an obvious improvement indeed!
> > However I tried my previous benchmark and TypedObject is still slower than
> > JS object.
> > 
> > JS version:
> > http://wangyanxing.github.io/TSBenchmark/benchmark/class.html
> > Source:
> > https://github.com/wangyanxing/TSBenchmark/blob/gh-pages/benchmark/src.js
> > 
> > Typed Object version:
> > http://wangyanxing.github.io/TSBenchmark/benchmark/struct.html
> > Source:
> > https://github.com/wangyanxing/TSBenchmark/blob/gh-pages/benchmark/src_opt.js
> > 
> > 
> > May I have some hints from you guys? Thanks a lot!!
> 
> What times are you seeing, and what platform are you on?  With the patch now
> in the tree, I get times of 10-20% faster in the typed objects version when
> running in the shell (i.e. after stripping out uses of document and canvas)
> on x86 and x64.
https://hg.mozilla.org/mozilla-central/rev/e19def475b91
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla39
You need to log in before you can comment on or make changes to this bug.