686144 - Eliminate gc::MarkingDelay

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Igor Bukanov]

To implement delayed marking, that we use when the GC marking requirements exceeds the capacity of the normal marking stack, we use one extra word per GC arena. The word allows to link the arena with things that caused marking delay. However, due to layout inefficiencies the real cost with 1Mb GC chunks is about 0.4% and it grows to 1.6% if we change the GC chunk size to 256K, which is one of the options I consider for the bug 671702 to decrease our heap fragmentation. 

For example, with 256K chunks without the Chunk::markingDelays arrays we can fit 63 arenas into the chunk. 63 arenas and their marking bitmaps takes 256K - 64 bytes and those 64 bytes is enough to fit ChunkInfo. But it is not enough to also fit the array of 63 marking delay words. That results in only 62 available arenas or 1.6% overhead causing noticeable regression in v8 benchmark.

To avoid that we can pack the delayed marking linkage together with ArenaHead::allocKind. The arena pointers always has ArenaShift (12 currently) low bits cleared and that is enough to fit ArenaHeader::allocKind.

The bug depends on the 686017 bug as it would make access to ArenaHeader::allocKind slightly slower and without that bug it causes visible V8 regression.

Comment 1

[Igor Bukanov]

Attachment: v1

The patch eliminates separated Chunk::markingDelay array and merges its words with the arena header.

Comment 2

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Comment on attachment 559708 [details] [diff] [review]
v1

Review of attachment 559708 [details] [diff] [review]:
-----------------------------------------------------------------

::: js/src/jsgc.h
@@ +386,5 @@
> +     * stack.
> +     *
> +     * To minimize the ArenaHeader size we record the next delayed marking
> +     * linkage as arenaAddress() >> ArenaShift and pack it with the allocKind
> +     * field and hasDelayedMarking flag. We use 8 bites for the allocKind, not

bites->bits

@@ +392,5 @@
> +     * to access it.
> +     */
> +  public:
> +    size_t       hasDelayedMarking  : 1;
> +    size_t       nextDelayedMarking : JS_BITS_PER_WORD - 8 - 1;

Could you add a getter and a setter for nextDelayedMarking that do the bit shifts and casting?

Comment 3

[Igor Bukanov]

https://hg.mozilla.org/integration/mozilla-inbound/rev/104fb6df714f - landed with getter and setter change

Comment 4

[(no longer active)]

https://hg.mozilla.org/mozilla-central/rev/104fb6df714f