584688 - finalizing strings after the GC

Status: RESOLVED DUPLICATE of bug 627200

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

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.

Comment 1

[Igor Bukanov]

Attachment: benchmark to measure string finalization speed

Comment 2

[Igor Bukanov]

Attachment: v1

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.

Comment 3

[Igor Bukanov]

Nominating for 2.0 - this should help dromaeo dom-attr tests among others as reported in bug 584286 comment 0.

Comment 4

[Igor Bukanov]

Attachment: v2

rebased patch. It applies on top of the patch from bug 589771 comment 2 on top of TM e2e1ea2a39ce.

Comment 5

[Igor Bukanov]

Attachment: v3

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.

Comment 6

[Igor Bukanov]

Attachment: v4

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.

Comment 7

[Igor Bukanov]

Attachment: v5

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.

Comment 8

[Robert Sayre]

we are going to land bug 558861 prior to this, so this patch is going to rot badly.