941311 - Improve GGC pretenuring heuristics

Status: RESOLVED FIXED

(Core :: JavaScript Engine: JIT, defect)

Description

[Brian Hackett [Laid off!]]

Attachment: patch

The way we determine whether objects should be pretenured has some problems and doesn't work reliably on octane-splay.  The main issue is that only 'new' scripts are pretenured, and not object/array initializers.  Only a small fraction of the objects octane-splay creates are such 'new' script nodes (splay nodes) --- the vast majority are in the payload trees and are currently in the nursery.  A secondary issue is that the heuristics are very fragile and trigger on other benchmarks as well.  The attached patch reworks the heuristics so that they only fire on splay, by only looking at what is promoted during single collections rather than the total number of times objects with a type have been promoted.  It also fixes things so that both 'new' scripts and initializer objects can be pretenured (only for JIT and VM cache hit allocations, atm), and fixes the invalidation mechanism used for recompiling code when types are marked as pretenured.

Comment 1

[Brian Hackett [Laid off!]]

Oh, testing splay individually this improves splay on x64 builds from ~7k to ~12-14k.  Trunk is ~14k.

Comment 2

[Terrence Cole [:terrence]]

Comment on attachment 8335618 [details] [diff] [review]
patch

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

r=me

::: js/src/gc/Nursery.cpp
@@ +393,5 @@
> +        return;
> +
> +    double promotionRate = trc->tenuredSize / double(allocationEnd() - start());
> +
> +    if (promotionRate < .8 && reason != JS::gcreason::FULL_STORE_BUFFER)

Maybe a comment explaining why this is important.

@@ +400,5 @@
> +    for (size_t i = 0; i < ArrayLength(tenureCounts); i++) {
> +        const TenureCount &entry = tenureCounts[i];
> +        if (entry.count >= 3000)
> +            pretenureTypes->append(entry.type); // ignore alloc failure
> +    }

Instead of copying promotion rate, how about we punch TenureCount[16] through and put this in the body of Nursery::collect?

Comment 3

[Jan de Mooij [:jandem]]

Comment on attachment 8335618 [details] [diff] [review]
patch

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

::: js/src/jit/IonBuilder.cpp
@@ +4578,5 @@
>      current->add(callObj);
>  
>      // Insert a post barrier to protect the following writes if we allocated
>      // the new call object directly into tenured storage.
> +    if (script()->treatAsRunOnce)

I was confused by this change, until I saw the script()->treatAsRunOnce a few lines above. Can you use callObj->needsSingletonType() here instead?

::: js/src/jsgc.cpp
@@ +4892,5 @@
> +    Nursery::TypeObjectList pretenureTypes;
> +    cx->runtime()->gcNursery.collect(cx->runtime(), reason, &pretenureTypes);
> +    for (size_t i = 0; i < pretenureTypes.length(); i++) {
> +        if (pretenureTypes[i]->canPreTenure())
> +            pretenureTypes[i]->setShouldPreTenure(cx);

Can we use JS_ALWAYS_TRUE? If not, will we leave a pending exception?

Comment 4

[Brian Hackett [Laid off!]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/5f093277a586

Comment 5

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/5f093277a586