tgui v4.4: More performance improvements #79943
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tgstation-server
added
the
UI
jlsnow301
added
Performance
|
this branch uses 2.2gb peak memory usage to clean compile the tgui target. the the Memory usage in prod matters because we almost always have some reason why page/swap/vertical ram has to be disabled, from both k8s and zfs hating it on linux to windows being stupid with it to byond's memory access patterns not meshing well with it to just pure disk iops budget concerns. Food for thoughts. |
If we used esbuild instead (which I'm not against), I'd like to still remove ie8 support from the esbuild branch, similar to this. It might even run faster as a result. Any objections to this? |
|
I have no objections to remove IE8 support and fully target IE11. Regarding node_modules: esbuild supports yarn pnp natively, evanw/esbuild#2451. Inferno only has a plugin for Babel, so I see no other option but to keep it just for transforming JSX, and esbuild would do the rest. |
|
Memory is already under pressure when there are like 5 keyholders inactive accounts logged in. |
I wonder if the higher memory usage is because we have to use the default node modules folder vs yarn's package management system. It doesn't make sense to me that swc would have higher memory usage than esbuild AND babel/terser |
This was referenced Nov 27, 2023
Merged
Mothblocks
pushed a commit
that referenced
this pull request
Nov 28, 2023
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> ## About The Pull Request We currently transpile TGUI down into ie8. This was primarily for Linux compatibility, which, to my understanding, hasn't worked for quite some time. As we approach 2024, consider that ie8 is nearly 15 years old and had its support ended in 2016. I believe sunsetting ie8 is in order. >I have no objections to remove IE8 support and fully target IE11. #79943 (comment) <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review and even discourage maintainers from merging your PR! --> ## Why It's Good For The Game - Probably improves performance to some degree, you're only as slow as your weakest link. - Reduced maintenance burden <!-- Argue for the merits of your changes and how they benefit the game, especially if they are controversial and/or far reaching. If you can't actually explain WHY what you are doing will improve the game, then it probably isn't good for the game in the first place. --> ## Changelog <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and it's effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> N/A if they can even see this message it doesn't affect them <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. -->
NaakaKo
pushed a commit
to Bird-Lounge/Skyraptor-SS13
that referenced
this pull request
Nov 28, 2023
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> ## About The Pull Request We currently transpile TGUI down into ie8. This was primarily for Linux compatibility, which, to my understanding, hasn't worked for quite some time. As we approach 2024, consider that ie8 is nearly 15 years old and had its support ended in 2016. I believe sunsetting ie8 is in order. >I have no objections to remove IE8 support and fully target IE11. tgstation/tgstation#79943 (comment) <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review and even discourage maintainers from merging your PR! --> ## Why It's Good For The Game - Probably improves performance to some degree, you're only as slow as your weakest link. - Reduced maintenance burden <!-- Argue for the merits of your changes and how they benefit the game, especially if they are controversial and/or far reaching. If you can't actually explain WHY what you are doing will improve the game, then it probably isn't good for the game in the first place. --> ## Changelog <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and it's effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> N/A if they can even see this message it doesn't affect them <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. -->
SkyratBot
pushed a commit
to Skyrat-SS13/Skyrat-tg
that referenced
this pull request
Nov 28, 2023
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> ## About The Pull Request We currently transpile TGUI down into ie8. This was primarily for Linux compatibility, which, to my understanding, hasn't worked for quite some time. As we approach 2024, consider that ie8 is nearly 15 years old and had its support ended in 2016. I believe sunsetting ie8 is in order. >I have no objections to remove IE8 support and fully target IE11. tgstation/tgstation#79943 (comment) <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review and even discourage maintainers from merging your PR! --> ## Why It's Good For The Game - Probably improves performance to some degree, you're only as slow as your weakest link. - Reduced maintenance burden <!-- Argue for the merits of your changes and how they benefit the game, especially if they are controversial and/or far reaching. If you can't actually explain WHY what you are doing will improve the game, then it probably isn't good for the game in the first place. --> ## Changelog <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and it's effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> N/A if they can even see this message it doesn't affect them <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. -->
vinylspiders
pushed a commit
to Skyrat-SS13/Skyrat-tg
that referenced
this pull request
Nov 28, 2023
* Drops ie8 support (#79974) <!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> ## About The Pull Request We currently transpile TGUI down into ie8. This was primarily for Linux compatibility, which, to my understanding, hasn't worked for quite some time. As we approach 2024, consider that ie8 is nearly 15 years old and had its support ended in 2016. I believe sunsetting ie8 is in order. >I have no objections to remove IE8 support and fully target IE11. tgstation/tgstation#79943 (comment) <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review and even discourage maintainers from merging your PR! --> ## Why It's Good For The Game - Probably improves performance to some degree, you're only as slow as your weakest link. - Reduced maintenance burden <!-- Argue for the merits of your changes and how they benefit the game, especially if they are controversial and/or far reaching. If you can't actually explain WHY what you are doing will improve the game, then it probably isn't good for the game in the first place. --> ## Changelog <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and it's effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> N/A if they can even see this message it doesn't affect them <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. --> * Drops ie8 support --------- Co-authored-by: Jeremiah <42397676+jlsnow301@users.noreply.github.com>
Iajret
pushed a commit
to Fluffy-Frontier/FluffySTG
that referenced
this pull request
Nov 28, 2023
* Drops ie8 support (#79974) <!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> We currently transpile TGUI down into ie8. This was primarily for Linux compatibility, which, to my understanding, hasn't worked for quite some time. As we approach 2024, consider that ie8 is nearly 15 years old and had its support ended in 2016. I believe sunsetting ie8 is in order. >I have no objections to remove IE8 support and fully target IE11. tgstation/tgstation#79943 (comment) <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review and even discourage maintainers from merging your PR! --> - Probably improves performance to some degree, you're only as slow as your weakest link. - Reduced maintenance burden <!-- Argue for the merits of your changes and how they benefit the game, especially if they are controversial and/or far reaching. If you can't actually explain WHY what you are doing will improve the game, then it probably isn't good for the game in the first place. --> <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and it's effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> N/A if they can even see this message it doesn't affect them <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. --> * Drops ie8 support --------- Co-authored-by: Jeremiah <42397676+jlsnow301@users.noreply.github.com>
lessthnthree
pushed a commit
to effigy-se/effigy-se
that referenced
this pull request
Nov 29, 2023
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> ## About The Pull Request We currently transpile TGUI down into ie8. This was primarily for Linux compatibility, which, to my understanding, hasn't worked for quite some time. As we approach 2024, consider that ie8 is nearly 15 years old and had its support ended in 2016. I believe sunsetting ie8 is in order. >I have no objections to remove IE8 support and fully target IE11. tgstation/tgstation#79943 (comment) <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review and even discourage maintainers from merging your PR! --> ## Why It's Good For The Game - Probably improves performance to some degree, you're only as slow as your weakest link. - Reduced maintenance burden <!-- Argue for the merits of your changes and how they benefit the game, especially if they are controversial and/or far reaching. If you can't actually explain WHY what you are doing will improve the game, then it probably isn't good for the game in the first place. --> ## Changelog <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and it's effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> N/A if they can even see this message it doesn't affect them <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. -->
Jolly-66
added a commit
to TaleStation/TaleStation
that referenced
this pull request
Nov 29, 2023
Original PR: tgstation/tgstation#79974 ----- ## About The Pull Request We currently transpile TGUI down into ie8. This was primarily for Linux compatibility, which, to my understanding, hasn't worked for quite some time. As we approach 2024, consider that ie8 is nearly 15 years old and had its support ended in 2016. I believe sunsetting ie8 is in order. >I have no objections to remove IE8 support and fully target IE11. tgstation/tgstation#79943 (comment) ## Why It's Good For The Game - Probably improves performance to some degree, you're only as slow as your weakest link. - Reduced maintenance burden ## Changelog N/A if they can even see this message it doesn't affect them --------- Co-authored-by: Jeremiah <42397676+jlsnow301@users.noreply.github.com> Co-authored-by: Jolly-66 <70232195+Jolly-66@users.noreply.github.com>
vvvv-vvvv
pushed a commit
to vvvv-vvvv/TerraGov-Marine-Corps
that referenced
this pull request
Dec 9, 2023
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> We currently transpile TGUI down into ie8. This was primarily for Linux compatibility, which, to my understanding, hasn't worked for quite some time. As we approach 2024, consider that ie8 is nearly 15 years old and had its support ended in 2016. I believe sunsetting ie8 is in order. >I have no objections to remove IE8 support and fully target IE11. tgstation/tgstation#79943 (comment) <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review and even discourage maintainers from merging your PR! --> - Probably improves performance to some degree, you're only as slow as your weakest link. - Reduced maintenance burden <!-- Argue for the merits of your changes and how they benefit the game, especially if they are controversial and/or far reaching. If you can't actually explain WHY what you are doing will improve the game, then it probably isn't good for the game in the first place. --> <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and it's effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> N/A if they can even see this message it doesn't affect them <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. --> (cherry picked from commit 60cf2629e5fee3cfa66ae1aefde191edb4675d3a)
Higgin
pushed a commit
to Higgin/tgstation
that referenced
this pull request
Dec 12, 2023
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- You can view Contributing.MD for a detailed description of the pull request process. --> ## About The Pull Request We currently transpile TGUI down into ie8. This was primarily for Linux compatibility, which, to my understanding, hasn't worked for quite some time. As we approach 2024, consider that ie8 is nearly 15 years old and had its support ended in 2016. I believe sunsetting ie8 is in order. >I have no objections to remove IE8 support and fully target IE11. tgstation#79943 (comment) <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review and even discourage maintainers from merging your PR! --> ## Why It's Good For The Game - Probably improves performance to some degree, you're only as slow as your weakest link. - Reduced maintenance burden <!-- Argue for the merits of your changes and how they benefit the game, especially if they are controversial and/or far reaching. If you can't actually explain WHY what you are doing will improve the game, then it probably isn't good for the game in the first place. --> ## Changelog <!-- If your PR modifies aspects of the game that can be concretely observed by players or admins you should add a changelog. If your change does NOT meet this description, remove this section. Be sure to properly mark your PRs to prevent unnecessary GBP loss. You can read up on GBP and it's effects on PRs in the tgstation guides for contributors. Please note that maintainers freely reserve the right to remove and add tags should they deem it appropriate. You can attempt to finagle the system all you want, but it's best to shoot for clear communication right off the bat. --> N/A if they can even see this message it doesn't affect them <!-- Both 🆑's are required for the changelog to work! You can put your name to the right of the first 🆑 if you want to overwrite your GitHub username as author ingame. --> <!-- You can use multiple of the same prefix (they're only used for the icon ingame) and delete the unneeded ones. Despite some of the tags, changelogs should generally represent how a player might be affected by the changes rather than a summary of the PR's contents. -->
Mothblocks
pushed a commit
that referenced
this pull request
Dec 15, 2023
## About The Pull Request Revival of #79943. I was not done with Babel. For some reason, swc was running very high memory. It was the minifier. If we use our current, only swapping out babel (the previous PR swapped the minifier AND transpiler), we not only keep the memory usage down to current levels but we **slash** TGUI build times. Look at these numbers! <details> <summary>Test environment / My specs</summary> intel i7-13700kf 32GB RAM windows 11 intel 660p m.2 SSD asus 4080 TUF series </details> <details> <summary>Benchmarks</summary> ### To perform these tests, build/install/compile once, then delete everything in the tgui/.yarn/webpack folder and all files in tgui/public **EXCEPT** tgui.html and tgui-polyfill.min.js Babel is our CURRENT. Build time  SWC is 64.36% faster than our current solution Peak memory usage  </details> What's the catch? Do we need to use node_modules now? No! ## Why It's Good For The Game Faster build tools! ## Changelog N/A nothing player facing
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
About The Pull Request
Consider this a much more intensive version of #79916. We're pretty limited by the speed of babel-loader, and this PR would completely replace it (with Rust!). Doing so removes the need for a minimizer, but it comes with drawbacks.
Test environment / My specs
intel i7-13700kf
32GB RAM
windows 11
intel 660p m.2 SSD
asus 4080 TUF series
Fresh build
Test steps:
tgui/.yarn/webpackfolderCTRL SHIFT BResults:
terser averages
25.16sswc averages
16.65s- 33.82% fasterCached build
(assuming files are already there, CTRL SHIFT B if not)
CTRL SHIFT BResults:
terser averages
9sswc averages
2.73s- 69.65% fasterThe resulting tgui build speed is faster than both #79916 and certainly our current babel/terser solution.
Why It's Good For The Game
Improved build speed, faster dev tools
Changelog
N/A nothing player facing