Closed Bug 594640 Opened 10 years ago Closed 9 years ago

TM: avoid redundant snapshots when recording GETELEM

Categories

(Core :: JavaScript Engine, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED

People

(Reporter: njn, Assigned: njn)

Details

(Whiteboard: fixed-in-tracemonkey)

Attachments

(1 file)

Pretty straightforward;  the patch just remembers a previous BRANCH_EXIT snapshot and forwards it onto a subsequent guard for reuse.  Cachegrind results:

---------------------------------------------------------------
| millions of instructions executed                           |
| total                        | on-trace (may overestimate)  |
---------------------------------------------------------------
|    99.976    99.181 (1.008x) |    25.815    25.810 (------) | 3d-cube
|    47.263    47.262 (------) |    25.562    25.562 (------) | 3d-morph
|   114.525   113.189 (1.012x) |    44.610    44.603 (------) | 3d-raytrace
|    68.899    68.899 (------) |    17.070    17.070 (------) | access-binary-
|   111.465   111.388 (1.001x) |    92.572    92.571 (------) | access-fannkuc
|    37.582    37.555 (1.001x) |    16.703    16.703 (------) | access-nbody
|    51.838    51.836 (------) |    31.633    31.633 (------) | access-nsieve
|    16.359    16.359 (------) |     3.249     3.249 (------) | bitops-3bit-bi
|    45.921    45.921 (------) |    32.702    32.702 (------) | bitops-bits-in
|    24.783    24.783 (------) |    12.019    12.019 (------) | bitops-bitwise
|    56.406    56.401 (------) |    42.156    42.156 (------) | bitops-nsieve-
|    34.434    34.434 (------) |    21.298    21.298 (------) | controlflow-re
|    47.361    47.275 (1.002x) |     6.175     6.174 (------) | crypto-md5
|    30.169    30.070 (1.003x) |     7.021     7.021 (------) | crypto-sha1
|    96.803    96.785 (------) |    17.536    17.536 (------) | date-format-to
|    83.656    83.649 (------) |     9.770     9.770 (------) | date-format-xp
|    54.289    54.286 (------) |    31.293    31.293 (------) | math-cordic
|    29.558    29.558 (------) |     6.232     6.232 (------) | math-partial-s
|    31.068    31.048 (1.001x) |    13.352    13.352 (------) | math-spectral-
|    58.534    58.534 (------) |    34.592    34.592 (------) | regexp-dna
|    39.763    39.760 (------) |     9.409     9.409 (------) | string-base64
|   116.829   116.829 (------) |    26.066    26.066 (------) | string-fasta
|   138.338   138.341 (------) |    17.895    17.895 (------) | string-tagclou
|   180.400   180.413 (------) |    22.160    22.160 (------) | string-unpack-
|    57.996    57.995 (------) |    11.858    11.858 (------) | string-validat
-------
|  1674.228  1671.765 (1.001x) |   578.760   578.747 (------) | all

It's a small win, but also a small and easy patch.
Attachment #473350 - Flags: review?(gal)
Attachment #473350 - Flags: review?(gal) → review+
Looks like this regressed Sunspider by 4.4ms in JM and 2ms in JM+TM on AFWY.
(In reply to comment #2)
> Looks like this regressed Sunspider by 4.4ms in JM and 2ms in JM+TM on AFWY.

I think that change is noise.

Instruction counts are a less ideal measurement than time, but they can be measured much more accurately -- the random variation from run to run is probably 1000x less than the variation for timings.

Comment 0 shows that the instruction counts were strictly reduced.  Looking at http://www.arewefastyet.com/individual.php?machine=6 I think ss-unpack is the only one that changed significantly;  it's hard to tell because that page doesn't give the revids on the graph.  It looks like ss-unpack may have had a flukily good result just prior to this patch landing.  The Cachegrind results show that ss-unpack (aka string-unpack-code) was unaffected by the patch.
http://hg.mozilla.org/mozilla-central/rev/7f7c60f205f6
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.