Closed
Bug 584688
Opened 14 years ago
Closed 14 years ago
finalizing strings after the GC
Categories
(Core :: JavaScript Engine, defect)
Core
JavaScript Engine
Tracking
()
RESOLVED
DUPLICATE
of bug 627200
People
(Reporter: igor, Assigned: igor)
References
(Blocks 1 open bug)
Details
Attachments
(3 files, 3 obsolete files)
(deleted),
text/plain
|
Details | |
(deleted),
patch
|
Details | Diff | Splinter Review | |
(deleted),
patch
|
Details | Diff | Splinter Review |
Even with offloading string finalization to the background thread string finalization is expensive as the GC still need to read fields of dead JSString to schedule later freeing of their char arrays. That can be avoided if do the sweeping phase for GC strings outside the GC either during allocation or on a backround thread.
Assignee | ||
Comment 1•14 years ago
|
||
Assignee | ||
Comment 2•14 years ago
|
||
Here is untested patch prototype. For the benchmark from the comment 1 I have on a dual-core CPU (Intel iCore 520):
before the patch:
({average_cycle_time:36.2, average_gc_time:1.18})
after the patch:
({average_cycle_time:33.6, average_gc_time:0.2})
So the patch speed ups the finalization by factor of 6 while reducing overall allocation/gc loop performance by 10% or so.
Attachment #470426 -
Attachment is obsolete: true
Assignee | ||
Comment 3•14 years ago
|
||
Nominating for 2.0 - this should help dromaeo dom-attr tests among others as reported in bug 584286 comment 0.
Assignee | ||
Comment 4•14 years ago
|
||
rebased patch. It applies on top of the patch from bug 589771 comment 2 on top of TM e2e1ea2a39ce.
Attachment #470427 -
Attachment is obsolete: true
Assignee | ||
Comment 5•14 years ago
|
||
In the new version I changed gc() for js shell to wait for the background finalization to finish. Without that the sunspider regressed with the patch for unpack-code for about 4.5% on my dual-core laptop. The reason is that sunspider shell driver has the following structure:
loop
load_test()
measure_test_time()
gc()
Now, with the background finalization the gc completes much earlier leading to the next test competing for system resources with the background thread. In the case of unpack-code this is rather significant as the benchmark is executed right after tagcloud with the latter creating a lot of string garbage.
Attachment #471084 -
Attachment is obsolete: true
Assignee | ||
Comment 6•14 years ago
|
||
rebased patch
With the patch arenas and chunks are freed in parallel with allocation. Thus using a vector of chunks with free arenas that was used previously no longer works. To minimize the source complexity I replaced that with a simple cache that just stores the last found chunk that has some free arenas.
Another change is that JSArenaList::cursor becomes a JSGCArena** pointer. This is to ensure an efficient insertion of finalized arenas into the free list. Note that currently the arenas are inserted into the free list after finalization of all the strings is done. With a lot of arenas to add to the free list this may potentially deny the allocation threads from useful arenas.
An alternative is to add the arenas immediately after finalization. But that may increase the GC lock contention. So I consider to do it later.
Attachment #473540 -
Attachment is obsolete: true
Attachment #473976 -
Flags: review?(anygregor)
Assignee | ||
Updated•14 years ago
|
Attachment #470426 -
Attachment is obsolete: false
Assignee | ||
Comment 7•14 years ago
|
||
The new patch adds to the shell gc() function support for not waiting for the background GC to finish via passing {nowait: true} parameter object. This should allow to benchmark finalization performance.
Attachment #473999 -
Flags: review?(anygregor)
Comment 8•14 years ago
|
||
we are going to land bug 558861 prior to this, so this patch is going to rot badly.
Updated•14 years ago
|
Assignee | ||
Updated•14 years ago
|
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → DUPLICATE
Updated•13 years ago
|
Attachment #473976 -
Flags: review?(anygregor)
Updated•13 years ago
|
Attachment #473999 -
Flags: review?(anygregor)
You need to log in
before you can comment on or make changes to this bug.
Description
•