Closed
Bug 1251488
Opened 9 years ago
Closed 1 year ago
ArenaHeader bool bit to record whether the arena contains any weakmap keys
Categories
(Core :: JavaScript: GC, defect)
Core
JavaScript: GC
Tracking
()
RESOLVED
WONTFIX
People
(Reporter: sfink, Assigned: sfink)
References
Details
Attachments
(2 files)
7.66 KB,
patch
|
Details | Diff | Splinter Review | |
1.48 KB,
text/plain
|
Details |
Rather than looking up every possible weak key while doing the weak marking phase, we could set a bit on the arena saying whether it contains any weakmap keys.
Assignee | ||
Comment 1•9 years ago
|
||
I implemented this, and saw basically no difference in timing. I even added measurements of how many times we stopped after checking the bit, and it happened 20 million times (out of about 20.1 million total lookups).
Without optimization: 1:52.81 (1 minute 52 seconds)
With optimization: 1:50.87
saves=20468588 hits=13870 misses=132400
saves: the arena bit was unset
hits: it was a weak map key
misses: arena bit was set but it wasn't a weak map key
Perhaps the lookup is fast? Or perhaps mapping a GCCellPtr to an arena header is slow? I don't know. Leaving this here for posterity.
Comment 2•9 years ago
|
||
That's a shame. I guess lookup is just really quick...
Assignee | ||
Comment 3•9 years ago
|
||
For the record, this is the benchmark test script I used to measure the behavior. It's a little crazy -- it creates a bit of a tangled-up graph and then GCs. Although I reported the total times above, I was mostly looking at the 'Weak Marking' output in MOZ_GCTIMER. And there was a lot of time being spent in there during the graph construction.
For the final GCs, I see:
original:
Mark Weak: 2205.529ms
Mark Weak: 2209.159ms
with optimization:
Mark Weak: 2131.709ms
Mark Weak: 2179.285ms
So (assuming these single runs are to be believed; I haven't looked at the variance) it's a 3% improvement on a workload that's heavy on weak marking, but only for this single phase.
Updated•3 years ago
|
Severity: normal → S3
Comment 4•1 year ago
|
||
Closing this as WONTFIX, merely because it has been evaluated and doesn't seem worth pushing forward given the results.
Status: ASSIGNED → RESOLVED
Closed: 1 year ago
Resolution: --- → WONTFIX
You need to log in
before you can comment on or make changes to this bug.
Description
•