704356 - Remove the property cache

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

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

[Boris Zbarsky [:bzbarsky]]

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...

Comment 2

[Nicholas Nethercote [inactive]]

I bet we could halve or even quarter the size of this with negligible effect, saving 60 KiB per process on B2G.

Comment 3

[Nicholas Nethercote [inactive]]

Attachment: Remove the property cache.

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.

Comment 4

[Nicholas Nethercote [inactive]]

Oh, and I tested debug mode too.  There's barely any slow-down, so the concerns in comment 1 appear to be addressed.

Comment 5

[Nicholas Nethercote [inactive]]

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.

Comment 6

[Nicholas Nethercote [inactive]]

With the removal of JM this might be more feasible.  See bug 868437 comment 0.

Comment 7

[Jan de Mooij [:jandem]]

Attachment: Patch

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.

Comment 8

[Nicholas Nethercote [inactive]]

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.

Comment 9

[Jan de Mooij [:jandem]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/d009ce8e199e

Comment 10

[Jan de Mooij [:jandem]]

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

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/d009ce8e199e
https://hg.mozilla.org/mozilla-central/rev/c86d40e96c89