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

tsc(perf): avoid large unions #30847

Closed
wants to merge 1 commit into from
Closed

tsc(perf): avoid large unions #30847

wants to merge 1 commit into from

Conversation

JonasBa
Copy link
Member

@JonasBa JonasBa commented Dec 27, 2021

This is a draft PR that will not get merged, it is posted just as a proof of concept

TLDR: we can cut TSC time almost in half and have a nicer and more responsive development experience

Longer story:
I wanted to understand why the typescript development flow on my machine was slow - code actions on save would take several seconds before type checking was done and errors due to incorrect types would pop-up, this was problematic because it decreased my development speed and resulted in multiple back and fourths between CI and TSC failing to build.

I've written down the investigation process and findings in https://www.notion.so/sentry/Typescript-performance-audit-ac375cf3d9374e66998a6eab2d5539e4

Before ~36s build time:
before
After ~21s
after

--diagnostics result before:

Files:              4008
Lines:            644694
Nodes:           2473488
Identifiers:      803265
Symbols:         3235184
Types:           1003633
Instantiations:  3280536
Memory used:    2772695K
I/O read:          0.46s
I/O write:         0.00s
Parse time:        2.93s
Bind time:         1.16s
Check time:       31.08s
Emit time:         0.00s
Total time:       35.16s

--diagnostics result after

Files:              4010 (+2)
Lines:            644522 (-172)
Nodes:           2468186 (-5302)
Identifiers:      801193 (-2072)
Symbols:         1627093 (-1 608 091)
Types:            540442 (-463 191)
Instantiations:  2499616 (-780 920
Memory used:    1725751K (-1 046 944K ~ 1GB)
I/O read:          0.43s (-0.03s)
I/O write:         0.00s (no change)
Parse time:        2.78s (-0.15s)
Bind time:         1.20s (+0.04s)
Check time:       17.15s (-13.93s)
Emit time:         0.00s (no change)
Total time:       21.13s (-14.03s)

The symbols, types, instantiations and memory usage are probably the most notable drops.

test

fix(tsc): fix large booleanfield union

fix(tsc): fix a few other components

test
@JonasBa JonasBa requested a review from a team December 28, 2021 09:43
@evanpurkhiser
Copy link
Member

Can't see the notion document right now, but wondering if we could split this out into a few different PRs for sets of changes?

@JonasBa
Copy link
Member Author

JonasBa commented Dec 28, 2021

Ah yes, sorry it was still private 🤦🏼‍♂️ - https://www.notion.so/sentry/Typescript-performance-audit-ac375cf3d9374e66998a6eab2d5539e4 I gave access to engineering

@@ -129,7 +131,7 @@ class BaseButton extends React.Component<ButtonProps, {}> {
// Doing this instead of using `Tooltip`'s `disabled` prop so that we can minimize snapshot nesting
if (title) {
return (
<Tooltip skipWrapper={!disabled} {...tooltipProps} title={title}>
Copy link
Member

@scttcper scttcper Jan 14, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we keep this, i think tooltips don't work on disabled buttons otherwise because of mouse events. maybe thats outdated, not sure.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@scttcper yes for sure, this branch is more of a proof of concept and we should pick whatever is useful from here. There’s a solid chance I went a bit overboard with some changes…

@github-actions
Copy link
Contributor

github-actions bot commented Feb 4, 2022

This pull request has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you label it Status: Backlog or Status: In Progress, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@github-actions github-actions bot closed this Feb 11, 2022
@github-actions github-actions bot locked and limited conversation to collaborators Feb 27, 2022
@mgaeta mgaeta deleted the jb/tsc-audit branch March 18, 2022 20:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants