Closed
Bug 704356
Opened 13 years ago
Closed 11 years ago
Remove the property cache
Categories
(Core :: JavaScript Engine, defect)
Tracking
()
RESOLVED
FIXED
mozilla24
People
(Reporter: n.nethercote, Assigned: jandem)
References
Details
Attachments
(1 file, 1 obsolete file)
40.17 KB,
patch
|
n.nethercote
:
review+
luke
:
superreview+
|
Details | Diff | Splinter Review |
Each property cache (we have one per ThreadData) is 128KB on 64-bit platforms and 80KB(?) on 32-bit platforms. bhackett reckons that once the trace JIT is gone, the property cache could probably be removed without hurting performance, because ICs do everything it does and faster. It's certainly worth measuring.
Comment 1•13 years ago
|
||
Do we need to turn on ICs in debug mode first? Or will code in debug mode run not-too-painfully-slowly (in the "debugging is impossible" sense) even without both ICs and the propcache? Also, we still hit the propcache from JM stubcalls right now; have we measured how much that helps? Seems like it helps some in cases we don't IC, which includes at least scripted getters/setters. Do we IC native setters now? I believe Brian fixed the lack of IC for native getters, at least...
Reporter | ||
Updated•13 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Reporter | ||
Updated•12 years ago
|
Assignee: n.nethercote → general
Reporter | ||
Updated•12 years ago
|
Status: ASSIGNED → NEW
Reporter | ||
Comment 2•12 years ago
|
||
I bet we could halve or even quarter the size of this with negligible effect, saving 60 KiB per process on B2G.
Blocks: slim-fast
Reporter | ||
Comment 3•11 years ago
|
||
This patch removes the property cache. The benefits are: - It saves 80 KiB (32-bit) or 160 KiB (64-bit) per JSRuntime. - A net reduction of 648 lines of code. Performance on Sunspider is marginally worse -- here are instruction counts. --------------------------------------------------------------- | millions of instructions executed | | total | compiled (may overestimate) | --------------------------------------------------------------- | 62.723 63.409 (0.989x) | 10.261 10.261 (------) | 3d-cube | 41.070 41.014 (1.001x) | 14.800 14.800 (------) | 3d-morph | 78.849 79.357 (0.994x) | 16.267 16.267 (------) | 3d-raytrace | 25.605 25.644 (0.998x) | 6.162 6.162 (------) | access-binary- | 49.802 49.819 (------) | 25.392 25.392 (------) | access-fannkuc | 26.817 27.044 (0.992x) | 6.979 6.979 (------) | access-nbody | 26.219 26.231 (------) | 10.594 10.594 (------) | access-nsieve | 18.476 18.420 (1.003x) | 4.344 4.344 (------) | bitops-3bit-bi | 28.969 28.914 (1.002x) | 14.866 14.866 (------) | bitops-bits-in | 25.333 25.278 (1.002x) | 11.642 11.642 (------) | bitops-bitwise | 32.833 32.778 (1.002x) | 17.619 17.619 (------) | bitops-nsieve- | 25.417 25.359 (1.002x) | 8.558 8.558 (------) | controlflow-re | 59.398 59.368 (1.001x) | 7.985 7.985 (------) | crypto-md5 | 32.178 32.153 (1.001x) | 8.255 8.255 (------) | crypto-sha1 | 83.562 84.300 (0.991x) | 14.159 14.159 (------) | date-format-to | 73.765 73.973 (0.997x) | 11.081 11.081 (------) | date-format-xp | 27.589 27.534 (1.002x) | 12.268 12.268 (------) | math-cordic | 41.401 41.460 (0.999x) | 7.671 7.671 (------) | math-partial-s | 23.754 23.700 (1.002x) | 6.521 6.521 (------) | math-spectral- | 62.566 62.502 (1.001x) | 35.228 35.228 (------) | regexp-dna | 40.332 40.576 (0.994x) | 7.902 7.902 (------) | string-base64 | 48.257 48.242 (------) | 18.199 18.199 (------) | string-fasta | 109.708 109.665 (------) | 17.870 17.870 (------) | string-tagclou | 125.195 130.479 (0.960x) | 11.920 11.920 (------) | string-unpack- | 50.791 50.880 (0.998x) | 9.333 9.333 (------) | string-validat ------- | 1220.620 1228.113 (0.994x) | 315.887 315.887 (------) | all string-unpack-code is a concern. Performance of the interpreter drops significantly in some cases. --------------------------------------------------------------- | millions of instructions executed | | total | compiled (may overestimate) | --------------------------------------------------------------- | 647.540 657.878 (0.984x) | 0 0 (------) | 3d-cube | 810.068 810.360 (------) | 0 0 (------) | 3d-morph | 617.472 724.905 (0.852x) | 0 0 (------) | 3d-raytrace | 336.932 419.794 (0.803x) | 0 0 (------) | access-binary- | 1877.117 1877.124 (------) | 0 0 (------) | access-fannkuc | 702.417 1008.521 (0.696x) | 0 0 (------) | access-nbody | 662.342 663.616 (0.998x) | 0 0 (------) | access-nsieve | 379.797 379.743 (------) | 0 0 (------) | bitops-3bit-bi | 633.762 633.708 (------) | 0 0 (------) | bitops-bits-in | 1966.864 1964.410 (1.001x) | 0 0 (------) | bitops-bitwise | 839.141 839.133 (------) | 0 0 (------) | bitops-nsieve- | 424.850 424.796 (------) | 0 0 (------) | controlflow-re | 350.294 356.953 (0.981x) | 0 0 (------) | crypto-md5 | 347.119 351.343 (0.988x) | 0 0 (------) | crypto-sha1 | 468.866 474.272 (0.989x) | 0 0 (------) | date-format-to | 240.213 267.553 (0.898x) | 0 0 (------) | date-format-xp | 830.113 830.058 (------) | 0 0 (------) | math-cordic | 642.885 669.910 (0.960x) | 0 0 (------) | math-partial-s | 451.234 451.183 (------) | 0 0 (------) | math-spectral- | 62.563 62.499 (1.001x) | 35.228 35.228 (------) | regexp-dna | 381.030 450.675 (0.845x) | 0 0 (------) | string-base64 | 594.365 606.277 (0.980x) | 0 0 (------) | string-fasta | 276.747 315.399 (0.877x) | 7.498 7.498 (------) | string-tagclou | 182.859 187.708 (0.974x) | 7.202 7.202 (------) | string-unpack- | 296.134 331.700 (0.893x) | 915264 915275 (------) | string-validat ------- | 15022.739 15759.527 (0.953x) | 52.264 52.264 (------) | all Perhaps we're not quite ready for this step yet. I'll try shrinking the table significantly instead, see how that goes.
Reporter | ||
Updated•11 years ago
|
Assignee: general → n.nethercote
Reporter | ||
Comment 4•11 years ago
|
||
Oh, and I tested debug mode too. There's barely any slow-down, so the concerns in comment 1 appear to be addressed.
Reporter | ||
Comment 5•11 years ago
|
||
I filed bug 843999 where I reduced the property cache's size by 16x without noticeable performance changes. So I'll remove the MemShrink tag from this bug, since going from a tiny table to no table won't affect memory consumption much.
Whiteboard: [MemShrink:P2]
Reporter | ||
Updated•11 years ago
|
Assignee: n.nethercote → general
Reporter | ||
Comment 6•11 years ago
|
||
With the removal of JM this might be more feasible. See bug 868437 comment 0.
Assignee | ||
Comment 7•11 years ago
|
||
Without the JITs, the property cache wins about 20 ms on SS (2982 -> 2962 ms), less than 1%. With the JITs, the difference is < 0.5 ms, it's hard to say more. The attached patch removes ~700 lines of code. Considering the property cache does not noticeably help performance anymore and is still causing bugs (for instance bug 821931), I think we should do this.
Assignee: general → jdemooij
Attachment #716951 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #763479 -
Flags: superreview?(luke)
Attachment #763479 -
Flags: review?(n.nethercote)
Updated•11 years ago
|
Attachment #763479 -
Flags: superreview?(luke) → superreview+
Reporter | ||
Comment 8•11 years ago
|
||
Comment on attachment 763479 [details] [diff] [review] Patch Review of attachment 763479 [details] [diff] [review]: ----------------------------------------------------------------- Your patch removed some stuff that I missed in my patch. Good. And bonus points for moving some code out of Interpreter-inl.h.
Attachment #763479 -
Flags: review?(n.nethercote) → review+
Assignee | ||
Comment 9•11 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/d009ce8e199e
Assignee | ||
Comment 10•11 years ago
|
||
For posterity, AWFY confirms this has no (negative) effect on benchmarks. There's a tiny (~0.5 ms) SS win on 32-bit/64-bit, but that's probably noise.
Comment 11•11 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d009ce8e199e https://hg.mozilla.org/mozilla-central/rev/c86d40e96c89
Status: ASSIGNED → RESOLVED
Closed: 11 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla24
You need to log in
before you can comment on or make changes to this bug.
Description
•