843999 - Shrink the property cache

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicholas Nethercote [inactive]]

In bug 704356 I tried removing the property cache.  It hurt the speed of the interpreter significantly, enough to make me leery.  But I found that shrinking it greatly doesn't cause the same problems.

Comment 1

[Nicholas Nethercote [inactive]]

Attachment: Shrink the property cache.

This patch reduces the size of the property cache from 4096 entries to 256
entries.  On 32-bit this reduces its size from 80 KiB to 5 KiB;  on 64-bit it
reduces the size from 160 KiB to 10 KiB.

Sunspider is barely affected when using the JITS:

---------------------------------------------------------------
| millions of instructions executed                           |
| total                        | compiled (may overestimate)  |
---------------------------------------------------------------
|    62.723    62.752 (------) |    10.261    10.261 (------) | 3d-cube
|    41.070    41.036 (1.001x) |    14.800    14.800 (------) | 3d-morph
|    78.849    78.836 (------) |    16.267    16.267 (------) | 3d-raytrace
|    25.605    25.591 (1.001x) |     6.162     6.162 (------) | access-binary-
|    49.802    49.770 (1.001x) |    25.392    25.392 (------) | access-fannkuc
|    26.817    26.790 (1.001x) |     6.979     6.979 (------) | access-nbody
|    26.219    26.253 (0.999x) |    10.594    10.594 (------) | access-nsieve
|    18.476    18.443 (1.002x) |     4.344     4.344 (------) | bitops-3bit-bi
|    28.969    28.935 (1.001x) |    14.866    14.866 (------) | bitops-bits-in
|    25.333    25.301 (1.001x) |    11.642    11.642 (------) | bitops-bitwise
|    32.833    32.800 (1.001x) |    17.619    17.619 (------) | bitops-nsieve-
|    25.417    25.382 (1.001x) |     8.558     8.558 (------) | controlflow-re
|    59.398    59.366 (1.001x) |     7.985     7.985 (------) | crypto-md5
|    32.178    32.144 (1.001x) |     8.255     8.255 (------) | crypto-sha1
|    83.562    83.935 (0.996x) |    14.159    14.159 (------) | date-format-to
|    73.765    73.729 (------) |    11.081    11.081 (------) | date-format-xp
|    27.589    27.556 (1.001x) |    12.268    12.268 (------) | math-cordic
|    41.401    41.367 (1.001x) |     7.671     7.671 (------) | math-partial-s
|    23.754    23.721 (1.001x) |     6.521     6.521 (------) | math-spectral-
|    62.566    62.512 (1.001x) |    35.228    35.228 (------) | regexp-dna
|    40.332    40.297 (1.001x) |     7.902     7.902 (------) | string-base64
|    48.257    48.225 (1.001x) |    18.199    18.199 (------) | string-fasta
|   109.707   109.651 (1.001x) |    17.870    17.870 (------) | string-tagclou
|   125.195   125.159 (------) |    11.920    11.920 (------) | string-unpack-
|    50.791    50.758 (1.001x) |     9.333     9.333 (------) | string-validat
-------
|  1220.619  1220.321 (------) |   315.887   315.887 (------) | all


nor when running just the interpreter:

---------------------------------------------------------------
| millions of instructions executed                           |
| total                        | compiled (may overestimate)  |
---------------------------------------------------------------
|   647.540   647.685 (------) |         0         0 (------) | 3d-cube
|   810.068   810.069 (------) |         0         0 (------) | 3d-morph
|   617.472   619.206 (0.997x) |         0         0 (------) | 3d-raytrace
|   336.932   339.950 (0.991x) |         0         0 (------) | access-binary-
|  1877.117  1877.084 (------) |         0         0 (------) | access-fannkuc
|   702.417   699.019 (1.005x) |         0         0 (------) | access-nbody
|   662.342   662.377 (------) |         0         0 (------) | access-nsieve
|   379.797   379.764 (------) |         0         0 (------) | bitops-3bit-bi
|   633.762   633.729 (------) |         0         0 (------) | bitops-bits-in
|  1966.864  1966.831 (------) |         0         0 (------) | bitops-bitwise
|   839.141   839.109 (------) |         0         0 (------) | bitops-nsieve-
|   424.850   424.817 (------) |         0         0 (------) | controlflow-re
|   350.294   350.253 (------) |         0         0 (------) | crypto-md5
|   347.119   347.080 (------) |         0         0 (------) | crypto-sha1
|   468.866   469.381 (0.999x) |         0         0 (------) | date-format-to
|   240.213   240.170 (------) |         0         0 (------) | date-format-xp
|   830.113   830.079 (------) |         0         0 (------) | math-cordic
|   642.885   642.851 (------) |         0         0 (------) | math-partial-s
|   451.234   451.200 (------) |         0         0 (------) | math-spectral-
|    62.563    62.510 (1.001x) |    35.228    35.228 (------) | regexp-dna
|   381.030   380.901 (------) |         0         0 (------) | string-base64
|   594.365   594.315 (------) |         0         0 (------) | string-fasta
|   276.747   276.675 (------) |     7.498     7.498 (------) | string-tagclou
|   182.859   182.843 (------) |     7.202     7.202 (------) | string-unpack-
|   296.136   296.079 (------) |    915264    915264 (------) | string-validat
-------
| 15022.741 15023.989 (------) |    52.264    52.264 (------) | all

Comment 2

[Nicholas Nethercote [inactive]]

jlebar: can memory reduction patches still be taken on B2G branches?  I know it's late, but this is a trivial change that saves 75 KiB per runtime without any downside.

The main B2G process usually has 5 runtimes (one main + four workers) and then each additional process usually has 1 runtime.  Given that there are usually 3 processes at start-up (I think?) that's a 525 KiB saving at start-up.

Comment 3

[Justin Lebar (not reading bugmail)]

(In reply to Nicholas Nethercote [:njn] from comment #2)
> jlebar: can memory reduction patches still be taken on B2G branches?  I know
> it's late, but this is a trivial change that saves 75 KiB per runtime
> without any downside.

We can likely take this on b2g18, but probably not b2g18_v1.0.1.

I'll follow up to your e-mail, but the two metrics we care about on b2g, I think, are:

 * How much memory can a fg app use, and
 * How many bg apps can we have running simultaneously.

We have explicitly asked partners not to measure memory usage at startup; it's not interesting for a variety of reasons, but in particular it does not necessarily correlate with the criteria above.  At startup we have various processes which will be killed when we're running low on memory, for example.

Anyway, the main process has four workers, iirc, so this is a savings of 300kb there alone (each worker gets its own runtime, right?).  I'd certainly like that.

Comment 4

[Nicholas Nethercote [inactive]]

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

Comment 5

[Nicholas Nethercote [inactive]]

Comment on attachment 717002 [details] [diff] [review]
Shrink the property cache.

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
Bug caused by (feature/regressing bug #): N/A

User impact if declined: Higher memory consumption: over 500 KiB at start-up, and at least 75 KiB per additional process.

Testing completed: just landed on mozilla-inbound.

Risk to taking this patch (and alternatives if risky): negligible.  The patch is trivial, it shrinks a single hash table.

String or UUID changes made by this patch: none.

Comment 6

[Nicholas Nethercote [inactive]]

> Anyway, the main process has four workers, iirc, so this is a savings of
> 300kb there alone (each worker gets its own runtime, right?).

As per comment 3: each process has N+1 runtimes, where N is the number of workers.  So, 375 KiB saved for the main process.

Comment 7

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/e48e1afb1899

Comment 8

[Milan Sreckovic [:milan] (needinfo for best results)]

Comment on attachment 717002 [details] [diff] [review]
Shrink the property cache.

This looks safe, as long as we don't have code elsewhere that uses 12 instead of SIZE_LOG2...

Comment 9

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/1c9db620b8ab

Is this something we'd consider uplifting to Aurora/Beta as well?

Comment 10

[Nicholas Nethercote [inactive]]

> Is this something we'd consider uplifting to Aurora/Beta as well?

I don't think it's necessary.  It's just a small memory consumption improvement (though it's bigger on B2G because of multi-process) and so is not a typical candidate for Aurora/Beta uplift.