Summary: | Drop the FTL(DFG) graph after lowering to B3 | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Robin Morisset <rmorisset> | ||||||||||||||||
Component: | JavaScriptCore | Assignee: | Robin Morisset <rmorisset> | ||||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||||
Severity: | Normal | CC: | commit-queue, ews-watchlist, fpizlo, keith_miller, mark.lam, msaboff, saam, tzagallo, webkit-bug-importer | ||||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||||
Hardware: | Unspecified | ||||||||||||||||||
OS: | Unspecified | ||||||||||||||||||
Bug Depends on: | 226749 | ||||||||||||||||||
Bug Blocks: | 227095 | ||||||||||||||||||
Attachments: |
|
Description
Robin Morisset
2021-06-02 15:40:29 PDT
Created attachment 430414 [details]
Patch
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. 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. Comment on attachment 430414 [details]
Patch
The Mac-wk2 bot has found a bug, I'm investigating it.
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.
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).
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. Tools/Scripts/svn-apply failed to apply attachment 430512 [details] to trunk.
Please resolve the conflicts and upload a new patch.
Created attachment 430555 [details]
Patch for landing
rebased
Patch 430555 does not build Created attachment 430580 [details]
Patch for landing
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]. Re-opened since this is blocked by bug 226749 Created attachment 430898 [details]
Patch for re-landing
Patch 430898 does not build Created attachment 431510 [details]
Patch for re-landing
rebased
Comment on attachment 431510 [details]
Patch for re-landing
Re-landing.
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]. |