Skip to content

Conversation

weswigham
Copy link
Member

@weswigham weswigham commented Oct 13, 2017

In this comment, I supposed that if we were concerned about the overhead of NodeLinks, we could reduce or remove it by inverting the shape of our cache structure. This is that.

This comparison was captured over 10 iterations on my machine (with a scanner exception for node and forced high power mode, as is usual for a windows machine):

Metric master reduce-cache-overhead Delta Best Worst
Compiler - Unions - node (v8.3.0, x64)
Memory used 203,010k (± 0.04%) 195,375k (± 0.04%) -7,636k (- 3.76%) 195,145k 195,530k
Parse Time 0.76s (± 3.91%) 0.73s (± 1.76%) -0.02s (- 3.03%) 0.72s 0.78s
Bind Time 0.66s (± 2.88%) 0.65s (± 0.72%) -0.01s (- 2.11%) 0.64s 0.66s
Check Time 10.19s (± 2.70%) 9.84s (± 1.81%) -0.35s (- 3.45%) 9.72s 10.54s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 11.61s (± 2.55%) 11.22s (± 1.56%) -0.39s (- 3.36%) 11.08s 11.91s
Monaco - node (v8.3.0, x64)
Memory used 331,677k (± 0.01%) 320,180k (± 0.02%) -11,497k (- 3.47%) 320,012k 320,301k
Parse Time 1.75s (± 1.32%) 1.73s (± 1.49%) -0.02s (- 1.03%) 1.70s 1.83s
Bind Time 0.70s (± 1.72%) 0.69s (± 1.22%) -0.01s (- 0.86%) 0.68s 0.71s
Check Time 3.69s (± 0.98%) 3.67s (± 1.16%) -0.02s (- 0.54%) 3.60s 3.79s
Emit Time 2.39s (± 1.20%) 2.38s (± 1.72%) -0.02s (- 0.75%) 2.31s 2.53s
Total Time 8.53s (± 0.73%) 8.47s (± 0.68%) -0.06s (- 0.73%) 8.37s 8.63s
TFS - node (v8.3.0, x64)
Memory used 288,481k (± 0.02%) 274,252k (± 0.02%) -14,229k (- 4.93%) 274,172k 274,334k
Parse Time 1.26s (± 1.99%) 1.24s (± 1.23%) -0.02s (- 1.19%) 1.23s 1.30s
Bind Time 0.71s (± 2.84%) 0.70s (± 0.68%) -0.01s (- 1.54%) 0.69s 0.71s
Check Time 3.25s (± 1.83%) 3.18s (± 0.96%) -0.07s (- 2.18%) 3.13s 3.27s
Emit Time 2.23s (± 2.55%) 2.11s (± 0.86%) -0.11s (- 4.99%) 2.07s 2.15s
Total Time 7.46s (± 1.86%) 7.25s (± 0.64%) -0.21s (- 2.79%) 7.15s 7.37s

There is consistently lower memory usage (since just add entries to spare arrays rather than allocate objects), and possible check time savings, too (since we no longer use a polymorphic NodeLinks object).

I'll try applying the same technique to SymbolLinks and see what the improvement there is.

@weswigham
Copy link
Member Author

Moving symbol links to the same layout yields slight further improvements:

Metric master reduce-cache-overhead Delta Best Worst
Compiler - Unions - node (v8.3.0, x64)
Memory used 203,010k (± 0.04%) 194,208k (± 0.03%) -8,802k (- 4.34%) 194,075k 194,315k
Parse Time 0.76s (± 3.91%) 0.73s (± 1.03%) -0.02s (- 3.17%) 0.72s 0.75s
Bind Time 0.66s (± 2.88%) 0.64s (± 1.29%) -0.02s (- 2.86%) 0.63s 0.66s
Check Time 10.19s (± 2.70%) 9.79s (± 0.52%) -0.40s (- 3.94%) 9.70s 9.93s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 11.61s (± 2.55%) 11.17s (± 0.47%) -0.44s (- 3.82%) 11.08s 11.31s
Monaco - node (v8.3.0, x64)
Memory used 331,677k (± 0.01%) 316,668k (± 0.02%) -15,009k (- 4.53%) 316,528k 316,768k
Parse Time 1.75s (± 1.32%) 1.73s (± 0.26%) -0.02s (- 1.37%) 1.72s 1.74s
Bind Time 0.70s (± 1.72%) 0.69s (± 1.28%) -0.00s (- 0.43%) 0.68s 0.71s
Check Time 3.69s (± 0.98%) 3.66s (± 0.50%) -0.03s (- 0.84%) 3.61s 3.70s
Emit Time 2.39s (± 1.20%) 2.33s (± 0.71%) -0.06s (- 2.46%) 2.29s 2.37s
Total Time 8.53s (± 0.73%) 8.41s (± 0.27%) -0.12s (- 1.38%) 8.37s 8.47s
TFS - node (v8.3.0, x64)
Memory used 288,481k (± 0.02%) 271,032k (± 0.02%) -17,450k (- 6.05%) 270,839k 271,116k
Parse Time 1.26s (± 1.99%) 1.24s (± 0.52%) -0.02s (- 1.27%) 1.23s 1.25s
Bind Time 0.71s (± 2.84%) 0.71s (± 1.16%) 0.00s ( 0.00%) 0.70s 0.74s
Check Time 3.25s (± 1.83%) 3.20s (± 1.57%) -0.06s (- 1.75%) 3.09s 3.28s
Emit Time 2.23s (± 2.55%) 2.12s (± 0.98%) -0.10s (- 4.54%) 2.07s 2.16s
Total Time 7.46s (± 1.86%) 7.28s (± 0.91%) -0.18s (- 2.37%) 7.12s 7.41s

I also tried removing TransientSymbol (not committed here) (which was a bit more work, as I had to remove all direct references to transient symbol fields) and did see some better perf, but worse memory usage (so a tradeoff - less polymorphism, but more space required to store the information).

@typescript-bot
Copy link
Collaborator

Thanks for your contribution. This PR has not been updated in a while and cannot be automatically merged at the time being. For housekeeping purposes we are closing stale PRs. If you'd still like to continue working on this PR, please leave a message and one of the maintainers can reopen it.

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.

2 participants