Bug 226556 – Drop the FTL(DFG) graph after lowering to B3

• Component: JavaScriptCore
• Status: RESOLVED FIXED

[Robin Morisset]

It is a fairly simple memory saving, that may well be significant since it will last through all of B3 and Air.

[Robin Morisset]

Created attachment 430414 [details]
Patch

[Robin Morisset]

I do not have performance numbers for this yet, I am currently launching the relevant A/B tasks and will add a comment with the results here once they complete.

[Mark Lam]

Comment on attachment 430414 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=430414&action=review

> Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp:10140
> +                Checked<int32_t> offsetCount { 1 };

This appears to be unused.

[Robin Morisset]

Comment on attachment 430414 [details]
Patch

The Mac-wk2 bot has found a bug, I'm investigating it.

[Robin Morisset]

Created attachment 430428 [details]
Patch

Fixed a bug: I forgot disabling the freeing in the case where --useSamplingProfiler=1.
As it relies on PCToCodeOriginMapBuilder which contains code such as
```
DFG::Node* node = bitwise_cast<DFG::Node*>(originRange.origin.data());
        if (node)
            appendItem(originRange.label, node->origin.semantic);
```
, having freed all DFG::Nodes obviously lead to trouble.

[Robin Morisset]

Created attachment 430512 [details]
Patch

fix two bugs related to the DOMJIT: I had missed the fact that it uses the DOMJITSnippets in a late-running generator (so freeing them obviously crashes), and that FTL::SnippetParams was capturing a Node* (which was easy to fix as it was only using it for its CodeOrigin).

[Robin Morisset]

I got some RAMification results on an M1 MacBookAir for this patch. Listing all of the subtests with p<0.05 changes:
- Box2D: 1.66% better (p<0.02)
- ML: 0.84% better (p<0.05)
- OfflineAssembler: 4.78% better (p<0.02)
- coffeescript-wtb: regression of 0.71% ! (p<0.05)
- crypto-aes-sp: 2.42% better (p<0.02)
- crypto-md5-sp: 3.10% better (p<0.02)
- crypto-sha1-sp: 0.89% better (p<0.02)
- date-format-tofte-sp: regression of 0.27% ! (p<0.02)
- delta-blue: 2.56% better (p<0.02)
- early-boyer: 0.86% better (p<0.05)
- gbemu: 1.74% better (p<0.02)
- navier-stokes: 1.52% better (p<0.02)
- octane-code-load: 0.39% better (p<0.05)
- raytrace: 2.00% better (p<0.02)
- regex-dna-sp: 0.65% better (p<0.02)
- regexp: 2.85% better (p<0.02)
- stanford-crypto-pbkdf2: 0.91% better (p<0.02)
- stanford-crypto-sha256: 0.46% better (p<0.02)

So despite some noise (as I only got 5 iterations instead of 8 as planned as the performance bots have been flaky in the past few days), it seems to be a pretty clear win.

I'll try to land it once EWS is happy with it.

[EWS]

Tools/Scripts/svn-apply failed to apply attachment 430512 [details] to trunk.
Please resolve the conflicts and upload a new patch.

[Robin Morisset]

Created attachment 430555 [details]
Patch for landing

rebased

[EWS]

Patch 430555 does not build

[Robin Morisset]

Created attachment 430580 [details]
Patch for landing

[EWS]

Committed r278463 (238483@main): <https://commits.webkit.org/238483@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 430580 [details].

[Radar WebKit Bug Importer]

<rdar://problem/78871457>

[WebKit Commit Bot]

Re-opened since this is blocked by bug 226749

[Robin Morisset]

Created attachment 430898 [details]
Patch for re-landing

[EWS]

Patch 430898 does not build

[Robin Morisset]

Created attachment 431510 [details]
Patch for re-landing

rebased

[Robin Morisset]

Comment on attachment 431510 [details]
Patch for re-landing

Re-landing.

[EWS]

Committed r278945 (238875@main): <https://commits.webkit.org/238875@main>

All reviewed patches have been landed. Closing bug and clearing flags on attachment 431510 [details].