Closed
Bug 601066
Opened 14 years ago
Closed 14 years ago
JM: Eliminate ImmutableSync on x64
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: sstangl, Unassigned)
References
Details
(Whiteboard: fixed-in-tracemonkey)
Attachments
(1 file, 1 obsolete file)
5.26 KB,
patch
|
dvander
:
review+
|
Details | Diff | Splinter Review |
With the patch in bug 598839, x64 gets better syncing in the non-syncFancy() case. However, ImmutableSync is untouched, since it is essentially a minor reimplementation of FrameState semantics that would either have to be heavily modified or eliminated. Elimination is probably the better option -- we can get rid of ~250LOC and an entire class and all associated buffers and memory by being slightly more clever with register use.
Need a plan of attack and justification here. The original syncing code was "clever" - to get good register movement it had to be too clever, and it was unmaintainable. We were constantly tracking down edge cases where registers weren't synced or restored properly. With IM, we know exactly what problems need to be solved and they are nicely encapsulated there. If the problems (which weren't described in this bug) are x64 specific, then an x64-specific ImmutableSync could be the answer.
Reporter | ||
Comment 2•14 years ago
|
||
Changed bug title to 'Eliminate ImmutableSync on x64'. Since x64 can trivially do whole-Value loads and stores, and we have a ValueReg always available, it is very simple to never drop out of FrameState::sync(). The numbers below are from this code. ImmutableSync is not defined for 64-bit builds, since it is not necessary. A small amount of memory is saved. SunSpider is not affected: it's difficult to distinguish from noise. Raytrace and Richards are the only tests on which any change is noticeable. ============================================================================= ** TOTAL **: 1.011x as fast 3814.1ms +/- 0.8% 3774.3ms +/- 0.7% ============================================================================= v8: 1.011x as fast 3814.1ms +/- 0.8% 3774.3ms +/- 0.7% crypto: - 473.9ms +/- 1.2% 469.0ms +/- 1.1% deltablue: - 1083.8ms +/- 2.1% 1078.9ms +/- 2.0% earley-boyer: *1.015x as slow* 415.5ms +/- 0.7% 421.6ms +/- 0.6% raytrace: 1.045x as fast 406.6ms +/- 1.5% 389.1ms +/- 2.3% regexp: - 489.6ms +/- 0.7% 489.4ms +/- 0.5% richards: 1.031x as fast 642.3ms +/- 0.1% 622.8ms +/- 0.2% splay: ?? 302.4ms +/- 1.8% 303.5ms +/- 1.8%
Summary: JM: Eliminate ImmutableSync → JM: Eliminate ImmutableSync on x64
Reporter | ||
Comment 3•14 years ago
|
||
Applies on top of 598839.
Attachment #480373 -
Flags: review?(dvander)
Reporter | ||
Updated•14 years ago
|
Attachment #480373 -
Flags: review?(dvander)
Reporter | ||
Comment 4•14 years ago
|
||
Applies on top of last patch from bug 598839. If you would like to hijack this bug to separate FrameState between the different architectures, that would be fine -- otherwise, the split can be done in the next bug.
Attachment #480373 -
Attachment is obsolete: true
Attachment #481427 -
Flags: review?(dvander)
Updated•14 years ago
|
Attachment #481427 -
Flags: review?(dvander) → review+
Reporter | ||
Comment 5•14 years ago
|
||
http://hg.mozilla.org/tracemonkey/rev/ad0c80eacba7
Whiteboard: fixed-in-tracemonkey
Reporter | ||
Comment 6•14 years ago
|
||
Was backed out: http://hg.mozilla.org/tracemonkey/rev/7b1071bf66fb
Whiteboard: fixed-in-tracemonkey
Reporter | ||
Comment 7•14 years ago
|
||
And you shake it all about, http://hg.mozilla.org/tracemonkey/rev/d0fd3ff9722e
Whiteboard: fixed-in-tracemonkey
Comment 8•14 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/d0fd3ff9722e
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•