Skip to content
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

Improves tgui build speed #79916

Merged
merged 13 commits into from
Nov 28, 2023
Merged

Conversation

jlsnow301
Copy link
Member

@jlsnow301 jlsnow301 commented Nov 24, 2023

About The Pull Request

Swaps out our minimizer, Terser, with esbuildplugin. This is the engine behind vite. This is a pretty straight forward replacement, let's let the numbers do the talking:

Test environment / My specs

intel i7-13700kf
32GB RAM
windows 11
intel 660p m.2 SSD
asus 4080 TUF series

Fresh build

Test steps:

  1. Delete tgui/.yarn/webpack folder
  2. CTRL SHIFT B

Results:
fresh

terser averages 25.16s
esbuild averages 19.37s - 23% faster

Cached build

(assuming files are already there, CTRL SHIFT B if not)

  1. add a comment to an interface file
  2. CTRL SHIFT B

Results:
cached

terser averages 9s
esbuild averages 3.52s - 60.91% faster

Why It's Good For The Game

Faster dev environment

Changelog

N/A nothing player facing

@tgstation-server tgstation-server added the UI We make the game less playable, but with round edges label Nov 24, 2023
@LemonInTheDark LemonInTheDark added the Performance Uses the 32-bit address space and slow interpreter more effectively label Nov 24, 2023
tgui/package.json Outdated Show resolved Hide resolved
@MrStonedOne
Copy link
Member

Works on windows 7, but I noticed the inner node executable hangs around for 10 seconds with esbuild.exe running after the build task ends. are you sure your savings are just from it releasing control early?

On production tgstation-server does need to know when it can safely trust the compile process has fully completed, this won't be an issue right away because tgstation-server handles building byond, not juke, so there will be a second compilation step anyways that will keep cleanup from happening too early, but it is something to look into.

I did however notice that peak memory usage is lower, which will help with compilation on prod where we have tighter memory budgets for vm-all-the-things reasons.

@jlsnow301 jlsnow301 marked this pull request as ready for review November 25, 2023 23:41
@jlsnow301 jlsnow301 marked this pull request as draft November 26, 2023 04:12
@jlsnow301
Copy link
Member Author

jlsnow301 commented Nov 26, 2023

Drafting this as #79943 seems to be the superior option. If that PR doesn't work out, this is a good compromise.

@jlsnow301 jlsnow301 marked this pull request as ready for review November 26, 2023 04:44
@Mothblocks Mothblocks added the 📌 Test Merge Candidate This version will not be removed by actions when the PR is updated label Nov 26, 2023
Copy link
Contributor

github-actions bot commented Nov 26, 2023

This pull request was test merged in 68 round(s).

Round list

manuel

sybil

@MrStonedOne
Copy link
Member

MrStonedOne commented Nov 27, 2023

mirroring this here:

the swc-loader branch uses 2.2gb peak memory usage to clean compile the tgui target.

the npm-updates branch used 1.8gb peak memory usage to clean compile the tgui target, and spent much less time at peak. This mostly matches with that the servers normally expect

the esbuild-plugin branch (this one) used 0.9gb peak memory usage to clean compile the tgui target.

This pr cuts memory usage during compilation massively.

github-actions bot added a commit that referenced this pull request Nov 28, 2023
@jlsnow301 jlsnow301 deleted the esbuild-plugin branch November 28, 2023 01:23
Jolly-66 added a commit to TaleStation/TaleStation that referenced this pull request Nov 29, 2023
Original PR: tgstation/tgstation#79916
-----
## About The Pull Request
Swaps out our minimizer, Terser, with esbuildplugin. This is the engine
behind vite. This is a pretty straight forward replacement, let's let
the numbers do the talking:


<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>Fresh build</summary>

Test steps:

1. Delete `tgui/.yarn/webpack` folder
2. `CTRL SHIFT B`

Results:

![fresh](https://github.com/tgstation/tgstation/assets/42397676/f337366f-2260-41b7-9ad6-f81f6f78e9e7)

terser averages `25.16s`
esbuild averages `19.37s` - 23% faster

</details>

<details>
<summary>Cached build</summary>

(assuming files are already there, CTRL SHIFT B if not)

1. add a comment to an interface file
2. `CTRL SHIFT B`

Results:

![cached](https://github.com/tgstation/tgstation/assets/42397676/89dc1c4b-a5b4-436d-a7e2-86683b6168f4)

terser averages `9s`
esbuild averages `3.52s` - 60.91% faster

</details>

## Why It's Good For The Game
Faster dev environment
## Changelog
N/A nothing player facing

Co-authored-by: Jeremiah <42397676+jlsnow301@users.noreply.github.com>
Co-authored-by: Jolly-66 <70232195+Jolly-66@users.noreply.github.com>
github-merge-queue bot pushed a commit to cmss13-devs/cmss13 that referenced this pull request May 1, 2024
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may
not be viewable. -->

# About the pull request
Replaces tgui build tools with faster alternatives.

There were some issues in build of components using hooks which do not
exist, which I removed as well. This is not a required part of the PR
but React does not use these

Based on two PRs: 
[Esbuild](tgstation/tgstation#79916)
[SWC](tgstation/tgstation#80310)

<!-- Remove this text and explain what the purpose of your PR is.

Mention if you have tested your changes. If you changed a map, make sure
you used the mapmerge tool.
If this is an Issue Correction, you can type "Fixes Issue #169420" to
link the PR to the corresponding Issue number #169420.

Remember: something that is self-evident to you might not be to others.
Explain your rationale fully, even if you feel it goes without saying.
-->

# Explain why it's good for the game
Faster build times
<!-- Please add a short description of why you think these changes would
benefit the game. If you can't justify it in words, it might not be
worth adding, and may discourage maintainers from reviewing or merging
your PR. This section is not strictly required for (non-controversial)
fix PRs or backend PRs. -->


# Testing Photographs and Procedure
<!-- Include any screenshots/videos/debugging steps of the modified code
functioning successfully, ideally including edge cases. -->


<!-- !! If you are modifying sprites, you **must** include one or more
in-game screenshots or videos of the new sprites. !! -->

<details>
<summary>Screenshots & Videos</summary>

Before
![Screenshot 2024-04-26
155554](https://github.com/cmss13-devs/cmss13/assets/42397676/5a48687e-dc6e-43d7-98d9-354bd3848ff6)

After
![Screenshot 2024-04-26
160516](https://github.com/cmss13-devs/cmss13/assets/42397676/b106ad86-25db-4403-b80a-29f4cd1ca38d)

</details>
Absolucy pushed a commit to Absolucy/Monkestation that referenced this pull request Jun 28, 2024
Swaps out our minimizer, Terser, with esbuildplugin. This is the engine
behind vite. This is a pretty straight forward replacement, let's let
the numbers do the talking:

<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>Fresh build</summary>

Test steps:

1. Delete `tgui/.yarn/webpack` folder
2. `CTRL SHIFT B`

Results:

![fresh](https://github.com/tgstation/tgstation/assets/42397676/f337366f-2260-41b7-9ad6-f81f6f78e9e7)

terser averages `25.16s`
esbuild averages `19.37s` - 23% faster

</details>

<details>
<summary>Cached build</summary>

(assuming files are already there, CTRL SHIFT B if not)

1. add a comment to an interface file
2. `CTRL SHIFT B`

Results:

![cached](https://github.com/tgstation/tgstation/assets/42397676/89dc1c4b-a5b4-436d-a7e2-86683b6168f4)

terser averages `9s`
esbuild averages `3.52s` - 60.91% faster

</details>

Faster dev environment
N/A nothing player facing
Absolucy pushed a commit to Absolucy/Monkestation that referenced this pull request Jun 28, 2024
Swaps out our minimizer, Terser, with esbuildplugin. This is the engine
behind vite. This is a pretty straight forward replacement, let's let
the numbers do the talking:

<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>Fresh build</summary>

Test steps:

1. Delete `tgui/.yarn/webpack` folder
2. `CTRL SHIFT B`

Results:

![fresh](https://github.com/tgstation/tgstation/assets/42397676/f337366f-2260-41b7-9ad6-f81f6f78e9e7)

terser averages `25.16s`
esbuild averages `19.37s` - 23% faster

</details>

<details>
<summary>Cached build</summary>

(assuming files are already there, CTRL SHIFT B if not)

1. add a comment to an interface file
2. `CTRL SHIFT B`

Results:

![cached](https://github.com/tgstation/tgstation/assets/42397676/89dc1c4b-a5b4-436d-a7e2-86683b6168f4)

terser averages `9s`
esbuild averages `3.52s` - 60.91% faster

</details>

Faster dev environment
N/A nothing player facing
dwasint pushed a commit to Monkestation/Monkestation2.0 that referenced this pull request Jun 29, 2024
Swaps out our minimizer, Terser, with esbuildplugin. This is the engine
behind vite. This is a pretty straight forward replacement, let's let
the numbers do the talking:

<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>Fresh build</summary>

Test steps:

1. Delete `tgui/.yarn/webpack` folder
2. `CTRL SHIFT B`

Results:

![fresh](https://github.com/tgstation/tgstation/assets/42397676/f337366f-2260-41b7-9ad6-f81f6f78e9e7)

terser averages `25.16s`
esbuild averages `19.37s` - 23% faster

</details>

<details>
<summary>Cached build</summary>

(assuming files are already there, CTRL SHIFT B if not)

1. add a comment to an interface file
2. `CTRL SHIFT B`

Results:

![cached](https://github.com/tgstation/tgstation/assets/42397676/89dc1c4b-a5b4-436d-a7e2-86683b6168f4)

terser averages `9s`
esbuild averages `3.52s` - 60.91% faster

</details>

Faster dev environment
N/A nothing player facing

Co-authored-by: Jeremiah <42397676+jlsnow301@users.noreply.github.com>
Doubleumc pushed a commit to Doubleumc/PvE-CMSS13 that referenced this pull request Jul 24, 2024
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may
not be viewable. -->

Replaces tgui build tools with faster alternatives.

There were some issues in build of components using hooks which do not
exist, which I removed as well. This is not a required part of the PR
but React does not use these

Based on two PRs:
[Esbuild](tgstation/tgstation#79916)
[SWC](tgstation/tgstation#80310)

<!-- Remove this text and explain what the purpose of your PR is.

Mention if you have tested your changes. If you changed a map, make sure
you used the mapmerge tool.
If this is an Issue Correction, you can type "Fixes Issue #169420" to
link the PR to the corresponding Issue number #169420.

Remember: something that is self-evident to you might not be to others.
Explain your rationale fully, even if you feel it goes without saying.
-->

Faster build times
<!-- Please add a short description of why you think these changes would
benefit the game. If you can't justify it in words, it might not be
worth adding, and may discourage maintainers from reviewing or merging
your PR. This section is not strictly required for (non-controversial)
fix PRs or backend PRs. -->

<!-- Include any screenshots/videos/debugging steps of the modified code
functioning successfully, ideally including edge cases. -->

<!-- !! If you are modifying sprites, you **must** include one or more
in-game screenshots or videos of the new sprites. !! -->

<details>
<summary>Screenshots & Videos</summary>

Before
![Screenshot 2024-04-26
155554](https://github.com/cmss13-devs/cmss13/assets/42397676/5a48687e-dc6e-43d7-98d9-354bd3848ff6)

After
![Screenshot 2024-04-26
160516](https://github.com/cmss13-devs/cmss13/assets/42397676/b106ad86-25db-4403-b80a-29f4cd1ca38d)

</details>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Performance Uses the 32-bit address space and slow interpreter more effectively 📌 Test Merge Candidate This version will not be removed by actions when the PR is updated UI We make the game less playable, but with round edges
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants