-
Notifications
You must be signed in to change notification settings - Fork 622
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Profile Guided Optimizations #10916
Comments
<p>This PR was automatically created by Snyk using the credentials of a real user.</p><br /><h3>Snyk has created this PR to upgrade react-router from 6.16.0 to 6.17.0.</h3> :information_source: Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project. <hr/> - The recommended version is **4 versions** ahead of your current version. - The recommended version was released **22 days ago**, on 2023-10-16. <details> <summary><b>Release notes</b></summary> <br/> <details> <summary>Package name: <b>react-router</b></summary> <ul> <li> <b>6.17.0</b> - <a href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.17.0">2023-10-16</a></br><p>[email protected]</p> </li> <li> <b>6.17.0-pre.2</b> - <a href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.17.0-pre.2">2023-10-13</a></br><p>[email protected]</p> </li> <li> <b>6.17.0-pre.1</b> - <a href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.17.0-pre.1">2023-10-12</a></br><p>[email protected]</p> </li> <li> <b>6.17.0-pre.0</b> - 2023-10-11 </li> <li> <b>6.16.0</b> - 2023-09-13 </li> </ul> from <a href="https://snyk.io/redirect/github/remix-run/react-router/releases">react-router GitHub release notes</a> </details> </details> <details> <summary><b>Commit messages</b></summary> </br> <details> <summary>Package name: <b>react-router</b></summary> <ul> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/edd9ad4957321cfb260cee21ad98aab2becfe250">edd9ad4</a> chore: Update version for release (near#10935)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/c1d0e50fc9ef5706c0d6ce9d0866ec1f4dadaab7">c1d0e50</a> Exit prerelease mode</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1c64bc1d4fe9c212dcd073b12ea51d2e10c45ea7">1c64bc1</a> Update readme for view transitions example</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1604c74f3abb0650910efb264908a3803fcc2e5e">1604c74</a> Split changeset for remix router and react-router-dom</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/ae843545c1a3a38c761b940ed5dc4fab15bb2d3a">ae84354</a> Update view-transitions example to use prerelease</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/6cfbd0e571018bf1d8722c09d70e394d2602f5be">6cfbd0e</a> chore: Update version for release (pre) (near#10934)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/c48341d6b75f4fd5b0ec60ed32c3c45ebb1e532f">c48341d</a> Lift startViewTransition implementation to react-router-dom (near#10928)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/b916689b4a211827cc324cf05994c334e25d380b">b916689</a> chore: Update version for release (pre) (near#10931)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/cbc9d7222cc4ca1e74f0b081472187bbd6a95a42">cbc9d72</a> Fix lint issues (near#10930)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1ad822c5bf8b32143aeef8511ca02577b487aafc">1ad822c</a> Update docs for startViewTransition (near#10927)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/e93e9088360e3fc1a4183efc5a39c8e680903554">e93e908</a> Docs updates</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/b09c5d09198b1ee4a8bfbf8a2a8910fc8eed7d2c">b09c5d0</a> chore: Update version for release (pre) (near#10924)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/d3203fb1b7bcfd73fa21e93b9b190defb769e33c">d3203fb</a> Enter prerelease mode</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/3adb639109ea5e90800e0b155035a610f0a09b4b">3adb639</a> Merge branch 'main' into release-next</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/a5451d5d3967a356e6d5af3cbefb858d2702044e">a5451d5</a> Update docs</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/feebfc0bf10614ba44ff43e2b9c69e22ad07a7a1">feebfc0</a> Add startViewTransition support (near#10916)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/7ce38dc49ee997706902ac2d033ba1fd683cfed0">7ce38dc</a> [Docs]: Use consistent feature warnings (near#10908)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/f77743aebfca26faabdd04e9ed1dd31721459877">f77743a</a> chore: format</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/8af53e7bfcf004916af4ea37e9d24e295d6ac107">8af53e7</a> Root router have a path different to '' or '/' (near#10852)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/ebe2491f7cd966d9967edb8acaeed86f9e1ab5b9">ebe2491</a> Fix RouterProvider future prop (near#10900)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/b98e82dbd774eadf3972f0b58f2542a8b5599d97">b98e82d</a> Specify `ErrorResponse` as interface to provide obvious contract (near#10876)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/da57748644da6400e2d051b2aa004df47beda1cf">da57748</a> fix(docs): add backticks to element names (near#10874)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/f8194fdb8e371b715d29d30a82e04a82a7648e9b">f8194fd</a> Handle case when session storage is blocked (near#10848)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/f9b3dbd9cbf513366c456b33d95227f42f36da63">f9b3dbd</a> chore: sort contributors list</li> </ul> <a href="https://snyk.io/redirect/github/remix-run/react-router/compare/13fb25a51184f66192e023e2e18be5ff00f37827...edd9ad4957321cfb260cee21ad98aab2becfe250">Compare</a> </details> </details> <hr/> **Note:** *You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs.* For more information: <img src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiIwMTQwNDNhYS1kNjYyLTQwMjMtOGQ5Yi02YzcyOTA0OTZjYmMiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6IjAxNDA0M2FhLWQ2NjItNDAyMy04ZDliLTZjNzI5MDQ5NmNiYyJ9fQ==" width="0" height="0"/> 🧐 [View latest project report](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763?utm_source=github&utm_medium=referral&page=upgrade-pr) 🛠 [Adjust upgrade PR settings](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763/settings/integration?utm_source=github&utm_medium=referral&page=upgrade-pr) 🔕 [Ignore this dependency or unsubscribe from future upgrade PRs](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763/settings/integration?pkg=react-router&utm_source=github&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades) <!--- (snyk:metadata:{"prId":"014043aa-d662-4023-8d9b-6c7290496cbc","prPublicId":"014043aa-d662-4023-8d9b-6c7290496cbc","dependencies":[{"name":"react-router","from":"6.16.0","to":"6.17.0"}],"packageManager":"npm","type":"auto","projectUrl":"https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763?utm_source=github&utm_medium=referral&page=upgrade-pr","projectPublicId":"98480bdc-d80b-4fd1-89d7-c4c56a706763","env":"prod","prType":"upgrade","vulns":[],"issuesToFix":[],"upgrade":[],"upgradeInfo":{"versionsDiff":4,"publishedDate":"2023-10-16T15:50:05.351Z"},"templateVariants":[],"hasFixes":false,"isMajorUpgrade":false,"isBreakingChange":false,"priorityScoreList":[]}) ---> Co-authored-by: snyk-bot <[email protected]>
<p>This PR was automatically created by Snyk using the credentials of a real user.</p><br /><h3>Snyk has created this PR to upgrade react-router-dom from 6.16.0 to 6.17.0.</h3> :information_source: Keep your dependencies up-to-date. This makes it easier to fix existing vulnerabilities and to more quickly identify and fix newly disclosed vulnerabilities when they affect your project. <hr/> - The recommended version is **4 versions** ahead of your current version. - The recommended version was released **22 days ago**, on 2023-10-16. <details> <summary><b>Release notes</b></summary> <br/> <details> <summary>Package name: <b>react-router-dom</b></summary> <ul> <li> <b>6.17.0</b> - <a href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.17.0">2023-10-16</a></br><p>[email protected]</p> </li> <li> <b>6.17.0-pre.2</b> - <a href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.17.0-pre.2">2023-10-13</a></br><p>[email protected]</p> </li> <li> <b>6.17.0-pre.1</b> - <a href="https://snyk.io/redirect/github/remix-run/react-router/releases/tag/react-router-native%406.17.0-pre.1">2023-10-12</a></br><p>[email protected]</p> </li> <li> <b>6.17.0-pre.0</b> - 2023-10-11 </li> <li> <b>6.16.0</b> - 2023-09-13 </li> </ul> from <a href="https://snyk.io/redirect/github/remix-run/react-router/releases">react-router-dom GitHub release notes</a> </details> </details> <details> <summary><b>Commit messages</b></summary> </br> <details> <summary>Package name: <b>react-router-dom</b></summary> <ul> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/edd9ad4957321cfb260cee21ad98aab2becfe250">edd9ad4</a> chore: Update version for release (near#10935)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/c1d0e50fc9ef5706c0d6ce9d0866ec1f4dadaab7">c1d0e50</a> Exit prerelease mode</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1c64bc1d4fe9c212dcd073b12ea51d2e10c45ea7">1c64bc1</a> Update readme for view transitions example</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1604c74f3abb0650910efb264908a3803fcc2e5e">1604c74</a> Split changeset for remix router and react-router-dom</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/ae843545c1a3a38c761b940ed5dc4fab15bb2d3a">ae84354</a> Update view-transitions example to use prerelease</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/6cfbd0e571018bf1d8722c09d70e394d2602f5be">6cfbd0e</a> chore: Update version for release (pre) (near#10934)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/c48341d6b75f4fd5b0ec60ed32c3c45ebb1e532f">c48341d</a> Lift startViewTransition implementation to react-router-dom (near#10928)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/b916689b4a211827cc324cf05994c334e25d380b">b916689</a> chore: Update version for release (pre) (near#10931)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/cbc9d7222cc4ca1e74f0b081472187bbd6a95a42">cbc9d72</a> Fix lint issues (near#10930)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/1ad822c5bf8b32143aeef8511ca02577b487aafc">1ad822c</a> Update docs for startViewTransition (near#10927)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/e93e9088360e3fc1a4183efc5a39c8e680903554">e93e908</a> Docs updates</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/b09c5d09198b1ee4a8bfbf8a2a8910fc8eed7d2c">b09c5d0</a> chore: Update version for release (pre) (near#10924)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/d3203fb1b7bcfd73fa21e93b9b190defb769e33c">d3203fb</a> Enter prerelease mode</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/3adb639109ea5e90800e0b155035a610f0a09b4b">3adb639</a> Merge branch 'main' into release-next</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/a5451d5d3967a356e6d5af3cbefb858d2702044e">a5451d5</a> Update docs</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/feebfc0bf10614ba44ff43e2b9c69e22ad07a7a1">feebfc0</a> Add startViewTransition support (near#10916)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/7ce38dc49ee997706902ac2d033ba1fd683cfed0">7ce38dc</a> [Docs]: Use consistent feature warnings (near#10908)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/f77743aebfca26faabdd04e9ed1dd31721459877">f77743a</a> chore: format</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/8af53e7bfcf004916af4ea37e9d24e295d6ac107">8af53e7</a> Root router have a path different to '' or '/' (near#10852)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/ebe2491f7cd966d9967edb8acaeed86f9e1ab5b9">ebe2491</a> Fix RouterProvider future prop (near#10900)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/b98e82dbd774eadf3972f0b58f2542a8b5599d97">b98e82d</a> Specify `ErrorResponse` as interface to provide obvious contract (near#10876)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/da57748644da6400e2d051b2aa004df47beda1cf">da57748</a> fix(docs): add backticks to element names (near#10874)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/f8194fdb8e371b715d29d30a82e04a82a7648e9b">f8194fd</a> Handle case when session storage is blocked (near#10848)</li> <li><a href="https://snyk.io/redirect/github/remix-run/react-router/commit/f9b3dbd9cbf513366c456b33d95227f42f36da63">f9b3dbd</a> chore: sort contributors list</li> </ul> <a href="https://snyk.io/redirect/github/remix-run/react-router/compare/13fb25a51184f66192e023e2e18be5ff00f37827...edd9ad4957321cfb260cee21ad98aab2becfe250">Compare</a> </details> </details> <hr/> **Note:** *You are seeing this because you or someone else with access to this repository has authorized Snyk to open upgrade PRs.* For more information: <img src="https://api.segment.io/v1/pixel/track?data=eyJ3cml0ZUtleSI6InJyWmxZcEdHY2RyTHZsb0lYd0dUcVg4WkFRTnNCOUEwIiwiYW5vbnltb3VzSWQiOiJlNTIxZTJlYi05MGNmLTRlZjEtYjljMC1iYTFlZTU2NjFjNzEiLCJldmVudCI6IlBSIHZpZXdlZCIsInByb3BlcnRpZXMiOnsicHJJZCI6ImU1MjFlMmViLTkwY2YtNGVmMS1iOWMwLWJhMWVlNTY2MWM3MSJ9fQ==" width="0" height="0"/> 🧐 [View latest project report](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763?utm_source=github&utm_medium=referral&page=upgrade-pr) 🛠 [Adjust upgrade PR settings](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763/settings/integration?utm_source=github&utm_medium=referral&page=upgrade-pr) 🔕 [Ignore this dependency or unsubscribe from future upgrade PRs](https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763/settings/integration?pkg=react-router-dom&utm_source=github&utm_medium=referral&page=upgrade-pr#auto-dep-upgrades) <!--- (snyk:metadata:{"prId":"e521e2eb-90cf-4ef1-b9c0-ba1ee5661c71","prPublicId":"e521e2eb-90cf-4ef1-b9c0-ba1ee5661c71","dependencies":[{"name":"react-router-dom","from":"6.16.0","to":"6.17.0"}],"packageManager":"npm","type":"auto","projectUrl":"https://app.snyk.io/org/ecp88/project/98480bdc-d80b-4fd1-89d7-c4c56a706763?utm_source=github&utm_medium=referral&page=upgrade-pr","projectPublicId":"98480bdc-d80b-4fd1-89d7-c4c56a706763","env":"prod","prType":"upgrade","vulns":[],"issuesToFix":[],"upgrade":[],"upgradeInfo":{"versionsDiff":4,"publishedDate":"2023-10-16T15:50:05.302Z"},"templateVariants":[],"hasFixes":false,"isMajorUpgrade":false,"isBreakingChange":false,"priorityScoreList":[]}) ---> Co-authored-by: snyk-bot <[email protected]>
Experiment reportTL;DRPGO is not useful right now. The LTO we already have enabled seems to not bring much benefits either actually. This is probably due to IO being a severe bottleneck in our current system, and thus should be revisited once we land memtrie. SummaryThis experiment report is the last in a series of experiments I performed, trying to see how much speedup PGO could actually get us. Unfortunately, trying to just enable PGO hits rust-lang/rust#115344. So we need to make a choice between LTO and PGO. Thin LTO does not work either. It turns out that, at the time of my writing this report, PGO actually does not get us anything — it’s even slightly slower than non-PGO’d binaries, even though in my test the workload is exactly what the profile was built on, so it should showcase the maximal benefits. My assumption is this is due to the fact that most of our time is actually spent in IO, and not computation. In particular, this means that optimizations from the compiler do not have a significant impact. As a consequence, while this experiment points towards not enabling PGO for now, we should revisit it once we have landed memtrie and IO is likely less of a bottleneck. At this time we can reuse the exact instructions described here to evaluate the impact. Incidentally, this experiment also led to me figuring out that LTO seems to only get us roughly 1% of performance on my benchmark. So it doesn’t actually seem to get us much, which also confirms the hypothesis that most of the time is currently spent doing IO. Setup
Compiling the baseline$ cargo clean
$ cargo build -p neard --release --target x86_64-unknown-linux-gnu
$ mv target/x86_64-unknown-linux-gnu/release/neard ../neard-base Compiling the binary that generates profile dataAs per the introductory paragraph, we need to change
Generating the profile data
Compiling the PGO’d binary
Evaluating the results
So PGO seems to have basically no significant impact on performance. Evaluating the impact of LTO removalAfter compiling a neard with LTO disabled:
This test was originally performed in order to figure out how big the benefits we could have once rust-lang/rust#115344 is fixed would be. However, it looks like neither LTO nor PGO actually bring much benefits currently. |
We should try to compile neard with PGO and see what gains there are to be had.
It might be very significant, just like it could be less than expected.
I’d still hope for a ~10% perf win, but it could be either more or less.
The text was updated successfully, but these errors were encountered: