Closed Bug 594655 Opened 14 years ago Closed 12 years ago

JM: same performance as non-JM builds on http://ben.adida.net/misc/bigint-javascript/

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set
normal

Tracking

()

RESOLVED FIXED

People

(Reporter: villalu, Unassigned)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

My friend Ben would like to do some crypto evoting stuff in Javascript, but is still doing it in Java, because he's getting two orders of magnitude faster performance in Java than in Javascript on similar code. His benchmark thing is here: http://ben.adida.net/misc/bigint-javascript/ After David's blog post, I was curious to see if the JM builds available here: http://nightly.mozilla.org/js-preview.html would help the scores, but I get essentially the same results (~9s) using 3.6, latest trunk nightlies, and latest JM nightlies on my OS X 10.6 box. So since he said we should file bugs on things that didn't improve... here I am :) I'll also ask Ben to cc himself to the bug.
63% under stubs::GetElem, which calls js_GetProperty, which calls js_GetPropertyHelper which does a resolve, etc. 21% stubs::SetElem calling js_SetProperty, etc. 12% in jit-generated symbol-less code.
So we're missing PICs at least. And presumably propcache is not being primed at all or something? Not sure what these objects are yet that GetElem/SetElem are sucking on.
Best guess based on code inspection so far is that they're BigInteger objects. So a vanilla jsobject.
Whatever Ben's running in Java it's not quite "similar code". The JS bigint library he's using is written in a very suboptimal way. And all the time is spent in the library, at least in Spidermonkey. If I change the library to use arrays for its array-like stuff, my time drops from about 1000ms per modexp to about 160ms per modexp. Interestingly, v8 is at about 100ms per modexp either way. After this change, 89% of the time is in JM-generated symbol-less code. 3% is in traced code, 2% is doing dense array ops from trace.
Attached file Modified version of jsbn.js (deleted) —
(In reply to comment #4) > Whatever Ben's running in Java it's not quite "similar code". Yes, that's true, I was mostly comparing FF to Chrome for this existing library. I'll check out your modified version of the library, which I haven't looked at in detail. (I'm using it as a library.)
Attached file Modified version of jsbn2.js (deleted) —
For what it's worth, Safari 5 here takes around 1300ms per modexp on the original page and 177ms per modexp on the modified bignum code.
And yeah, it would be interesting to know how v8 makes numbered props on random objects that fast... as well as how they make the array version still faster than us. ;)
(In reply to comment #4) > If I change the library to use arrays for its array-like stuff, my time drops > from about 1000ms per modexp to about 160ms per modexp. Interestingly, v8 is > at about 100ms per modexp either way. V8 makes a dense-array-like optimization universal for all objects -- we should too. I think there's a bug on this... bug 586842. /be
Ah, indeed. So we should fix that, and focus this bug on the modified js files?
Depends on: 586842
For what it is worth, this is down to ~5700ms on my box with the latest nightlies (still ~9000ms with Beta 6, so the code hasn't changed on Ben's side.) So something has improved, not sure what.
So if one actually uses arrays for array stuff, we kick the pants off Chrome and Safari (750ms vs 1300 and 1500 respectively for 10 modeexps). On the original site, we're about 6x slower than Chrome (and 3x faster than Safari). That's still the issue in the bug blocking this one.
Any progress on nightlies? /be
Seems we're behind, at least for Ben's application. Did V8 pull ahead since comment 12? https://twitter.com/#!/benadida/status/144499369117364225 /be
(In reply to Brendan Eich [:brendan] from comment #14) > Seems we're behind, at least for Ben's application. Did V8 pull ahead since > comment 12? > > https://twitter.com/#!/benadida/status/144499369117364225 > > /be Well, they added Crankshaft. Is there a link to the updated test? The URLs in this bug are 404s.
You can u(In reply to Brian Hackett (:bhackett) from comment #15) > Well, they added Crankshaft. Is there a link to the updated test? The URLs > in this bug are 404s. You can use the attachment with the modified code: https://bug594655.bugzilla.mozilla.org/attachment.cgi?id=489304 I get ~500ms on FF8, ~200ms on Chrome.
> You can use the attachment with the modified code: Which is completely different from the original code, notr; that was the point of modifying it. The original code used random objects with numeric indices (hence the dep on bug 586842). That was the primary performance effect there. The modified code uses arrays. On the modified code, I get 322ms in Chrome Dev, 688ms in Fx8 (JM+TM), and 375ms in a current nightly (JM+TI).
I get 280ms with a recent firefox nightly, 255ms with chrome 15. FF9 added type inference, which should help a good deal here. In the critical loop, the 'this.arr' and 'w.arr' accesses are not getting hoisted, and it looks like we should be able to hoist them. This may be a performance bug in the engine. The arrays are also not known to be packed (no missing holes in the middle), if you initialize the 'arr' arrays from the bottom up (write to a[0..n] before a[n+1]) then things should speed up, though I don't know how practical that is to do here.
At least on Windows 7, this is now faster in F15 (284 ms) than latest Chrome (350 ms). I'm going to close FIXED.
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: