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

Convert node and symbol links into checker-local sparse arrays with per-property getters and setters #28403

Closed
wants to merge 1 commit into from

Conversation

weswigham
Copy link
Member

Was talking about this with @ahejlsberg the other day. Still to need rerun perf tests to verify that this still gets us some perf gains (I put up #19163 a long time ago, but we hesitated on it because the API was eh, despite the perf gains); but this API (thanks for the suggestions @ahejlsberg) is actually nicer than our older symbol links API (in addition to being monomorphic), since adding new ones is easy and doesn't require synchronizing types. I'll have some perf testing results up soon.

As an aside, while refactoring I discovered we actually had two unused NodeLinks members - resolvedIndexInfo and resolvedJsxElementAllAttributesType. They are simply gone, now.

@weswigham
Copy link
Member Author

At least in node 9.9, it looks like this no longer helps blanket performance, just memory usage (although 3-5% memory usage improvements are pretty nice):

Metric master cached-bacon Delta Best Worst
Compiler - node (v9.9.0, x64)
Memory used 189,180k (± 0.60%) 180,201k (± 0.05%) -8,979k (- 4.75%) 180,082k 180,532k
Parse Time 0.80s (± 6.76%) 0.84s (± 7.17%) +0.03s (+ 4.23%) 0.74s 1.03s
Bind Time 0.91s (± 2.64%) 0.95s (± 7.41%) +0.04s (+ 4.28%) 0.78s 1.12s
Check Time 3.42s (± 1.93%) 3.61s (± 3.67%) +0.19s (+ 5.68%) 3.39s 3.85s
Emit Time 2.65s (± 0.73%) 2.74s (± 3.35%) +0.09s (+ 3.36%) 2.58s 2.96s
Total Time 7.78s (± 1.36%) 8.14s (± 3.59%) +0.36s (+ 4.62%) 7.70s 8.84s
Compiler - Unions - node (v9.9.0, x64)
Memory used 216,173k (± 0.49%) 208,412k (± 0.49%) -7,761k (- 3.59%) 207,190k 210,574k
Parse Time 0.71s (± 2.28%) 0.71s (± 2.66%) -0.01s (- 0.84%) 0.67s 0.76s
Bind Time 0.86s (± 3.09%) 0.88s (± 2.39%) +0.02s (+ 2.69%) 0.85s 0.93s
Check Time 10.38s (± 0.61%) 10.24s (± 0.43%) -0.14s (- 1.35%) 10.17s 10.38s
Emit Time 2.46s (± 1.51%) 2.41s (± 0.87%) -0.05s (- 2.03%) 2.37s 2.47s
Total Time 14.42s (± 0.62%) 14.24s (± 0.39%) -0.17s (- 1.21%) 14.14s 14.36s
Monaco - node (v9.9.0, x64)
Memory used 356,892k (± 0.02%) 343,531k (± 0.02%) -13,362k (- 3.74%) 343,357k 343,673k
Parse Time 1.67s (± 2.47%) 1.76s (± 4.46%) +0.09s (+ 5.15%) 1.64s 1.96s
Bind Time 0.81s (± 1.64%) 0.81s (± 4.68%) -0.01s (- 0.62%) 0.68s 0.88s
Check Time 3.91s (± 1.14%) 4.00s (± 2.05%) +0.08s (+ 2.17%) 3.85s 4.12s
Emit Time 3.59s (± 1.90%) 3.75s (± 4.17%) +0.16s (+ 4.54%) 3.51s 4.33s
Total Time 9.98s (± 0.99%) 10.31s (± 1.89%) +0.33s (+ 3.28%) 9.85s 10.86s
TFS - node (v9.9.0, x64)
Memory used 312,920k (± 0.01%) 296,075k (± 0.01%) -16,846k (- 5.38%) 296,016k 296,186k
Parse Time 1.17s (± 0.70%) 1.17s (± 1.95%) 0.00s ( 0.00%) 1.14s 1.25s
Bind Time 0.59s (± 2.51%) 0.59s (± 0.99%) -0.01s (- 1.34%) 0.58s 0.60s
Check Time 3.70s (± 0.90%) 3.67s (± 0.92%) -0.03s (- 0.92%) 3.63s 3.79s
Emit Time 3.21s (± 2.75%) 3.16s (± 1.35%) -0.05s (- 1.65%) 3.08s 3.27s
Total Time 8.68s (± 1.18%) 8.58s (± 0.56%) -0.10s (- 1.13%) 8.44s 8.70s
skype4life - node (v9.9.0, x64)
Memory used 826,964k (± 0.00%) 800,319k (± 0.01%) -26,645k (- 3.22%) 800,125k 800,414k
Parse Time 4.71s (± 1.39%) 4.88s (± 4.80%) +0.17s (+ 3.66%) 4.61s 5.56s
Bind Time 1.46s (± 1.54%) 1.51s (± 7.24%) +0.05s (+ 3.50%) 1.43s 1.94s
Check Time 15.13s (± 1.07%) 15.41s (± 5.37%) +0.28s (+ 1.82%) 14.67s 18.64s
Emit Time 10.28s (± 0.83%) 10.15s (± 3.34%) -0.13s (- 1.26%) 9.53s 11.09s
Total Time 31.57s (± 0.72%) 31.94s (± 4.20%) +0.37s (+ 1.16%) 30.65s 37.23s

not really scientific since the iteration count is so low, though. Need to get the perf testing machine running again, I think. In any case, I'll try for some more stable numbers overnight.

@weswigham
Copy link
Member Author

Yeah, the node 11 runs on windows look quite a bit like the node 9 ones. Memory improvements, but no perf change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant