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.

Sign up for GitHub

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

Jump to bottom

Preserve source newlines all the things #37814

Closed
wants to merge 12 commits into from
Closed

Preserve source newlines all the things #37814

wants to merge 12 commits into from

Conversation

DanielRosenwasser
Copy link
Member

@DanielRosenwasser DanielRosenwasser commented Apr 6, 2020 •
edited
Loading

An experiment right now to see how the compiler does with preserving source newlines. Right now, the most interesting thing is seeing the bad emit - what is going wrong. The same issues might apply to refactorings/quick fixes, so we should understand what's going wrong and fix that.

Sorta fixes #843.

@DanielRosenwasser DanielRosenwasser mentioned this pull request Apr 6, 2020
Closed
@DanielRosenwasser
Copy link
Member Author

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Apr 6, 2020 •
edited
Loading

Heya @DanielRosenwasser, I've started to run the perf test suite on this PR at f2c0bbb. You can monitor the build here.

Update: The results are in!

@typescript-bot
Copy link
Collaborator

@DanielRosenwasser
The results of the perf run you requested are in!

Here they are:

Comparison Report - master..37814

Metric master 37814 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 327,557k (± 0.05%) 327,107k (± 0.02%) -450k (- 0.14%) 326,918k 327,299k
Parse Time 1.63s (± 0.54%) 1.63s (± 0.61%) -0.00s (- 0.12%) 1.61s 1.65s
Bind Time 0.89s (± 0.53%) 0.89s (± 0.45%) -0.00s (- 0.45%) 0.88s 0.90s
Check Time 4.76s (± 0.41%) 4.75s (± 0.40%) -0.01s (- 0.19%) 4.72s 4.81s
Emit Time 5.35s (± 0.84%) 5.65s (± 0.81%) +0.29s (+ 5.44%) 5.57s 5.78s
Total Time 12.63s (± 0.41%) 12.91s (± 0.38%) +0.27s (+ 2.16%) 12.79s 13.01s
Monaco - node (v10.16.3, x64)
Memory used 327,027k (± 0.02%) 327,074k (± 0.01%) +47k (+ 0.01%) 326,960k 327,176k
Parse Time 1.26s (± 0.44%) 1.26s (± 0.60%) +0.00s (+ 0.16%) 1.25s 1.28s
Bind Time 0.77s (± 0.84%) 0.78s (± 0.63%) +0.00s (+ 0.52%) 0.76s 0.78s
Check Time 4.75s (± 0.29%) 4.77s (± 0.34%) +0.02s (+ 0.51%) 4.73s 4.80s
Emit Time 2.94s (± 0.75%) 3.21s (± 0.56%) +0.27s (+ 9.23%) 3.17s 3.25s
Total Time 9.71s (± 0.28%) 10.02s (± 0.32%) +0.30s (+ 3.13%) 9.93s 10.09s
TFS - node (v10.16.3, x64)
Memory used 292,035k (± 0.01%) 292,129k (± 0.02%) +94k (+ 0.03%) 292,011k 292,261k
Parse Time 0.96s (± 0.60%) 0.96s (± 0.63%) -0.01s (- 0.83%) 0.94s 0.97s
Bind Time 0.75s (± 1.11%) 0.74s (± 0.91%) -0.00s (- 0.27%) 0.73s 0.76s
Check Time 4.30s (± 0.43%) 4.29s (± 0.36%) -0.01s (- 0.21%) 4.26s 4.32s
Emit Time 3.05s (± 0.58%) 3.49s (± 0.89%) +0.44s (+14.28%) 3.40s 3.55s
Total Time 9.06s (± 0.24%) 9.49s (± 0.35%) +0.42s (+ 4.66%) 9.42s 9.58s
material-ui - node (v10.16.3, x64)
Memory used 450,585k (± 0.01%) 450,287k (± 0.01%) -298k (- 0.07%) 450,177k 450,365k
Parse Time 1.77s (± 0.27%) 1.78s (± 0.38%) +0.00s (+ 0.17%) 1.77s 1.80s
Bind Time 0.68s (± 0.59%) 0.69s (± 0.53%) +0.01s (+ 1.03%) 0.68s 0.69s
Check Time 12.55s (± 0.40%) 12.57s (± 0.52%) +0.02s (+ 0.14%) 12.42s 12.70s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.00s (± 0.33%) 15.03s (± 0.45%) +0.03s (+ 0.18%) 14.88s 15.16s
Angular - node (v12.1.0, x64)
Memory used 303,208k (± 0.02%) 302,565k (± 0.07%) -644k (- 0.21%) 301,762k 302,923k
Parse Time 1.59s (± 0.52%) 1.58s (± 0.33%) -0.01s (- 0.44%) 1.57s 1.59s
Bind Time 0.88s (± 1.02%) 0.88s (± 0.65%) +0.00s (+ 0.46%) 0.87s 0.89s
Check Time 4.65s (± 0.47%) 4.68s (± 0.79%) +0.02s (+ 0.49%) 4.62s 4.79s
Emit Time 5.54s (± 1.12%) 5.88s (± 0.48%) +0.34s (+ 6.09%) 5.82s 5.93s
Total Time 12.65s (± 0.48%) 13.01s (± 0.41%) +0.36s (+ 2.83%) 12.93s 13.15s
Monaco - node (v12.1.0, x64)
Memory used 307,030k (± 0.02%) 307,030k (± 0.01%) 0k ( 0.00%) 306,940k 307,133k
Parse Time 1.22s (± 0.61%) 1.22s (± 0.51%) +0.00s (+ 0.33%) 1.21s 1.24s
Bind Time 0.75s (± 0.87%) 0.75s (± 0.91%) -0.00s (- 0.27%) 0.73s 0.76s
Check Time 4.58s (± 0.48%) 4.57s (± 0.51%) -0.01s (- 0.26%) 4.53s 4.63s
Emit Time 2.96s (± 0.53%) 3.32s (± 0.67%) +0.37s (+12.42%) 3.29s 3.38s
Total Time 9.50s (± 0.27%) 9.86s (± 0.41%) +0.35s (+ 3.70%) 9.80s 9.97s
TFS - node (v12.1.0, x64)
Memory used 274,344k (± 0.02%) 274,452k (± 0.02%) +108k (+ 0.04%) 274,320k 274,667k
Parse Time 0.93s (± 0.83%) 0.94s (± 0.94%) +0.01s (+ 0.64%) 0.92s 0.96s
Bind Time 0.70s (± 0.68%) 0.71s (± 0.97%) +0.00s (+ 0.28%) 0.69s 0.72s
Check Time 4.19s (± 0.46%) 4.23s (± 0.39%) +0.04s (+ 0.96%) 4.18s 4.26s
Emit Time 3.08s (± 0.83%) 3.59s (± 0.62%) +0.51s (+16.54%) 3.55s 3.66s
Total Time 8.90s (± 0.49%) 9.45s (± 0.32%) +0.55s (+ 6.22%) 9.40s 9.55s
material-ui - node (v12.1.0, x64)
Memory used 427,946k (± 0.06%) 427,657k (± 0.05%) -289k (- 0.07%) 426,838k 427,999k
Parse Time 1.75s (± 0.44%) 1.76s (± 0.51%) +0.00s (+ 0.29%) 1.73s 1.77s
Bind Time 0.63s (± 0.53%) 0.64s (± 0.91%) +0.01s (+ 1.11%) 0.62s 0.65s
Check Time 11.23s (± 0.63%) 11.29s (± 0.98%) +0.06s (+ 0.53%) 11.05s 11.54s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 13.62s (± 0.53%) 13.69s (± 0.87%) +0.07s (+ 0.51%) 13.43s 13.95s
Angular - node (v8.9.0, x64)
Memory used 322,522k (± 0.02%) 322,037k (± 0.02%) -485k (- 0.15%) 321,908k 322,152k
Parse Time 2.12s (± 0.47%) 2.11s (± 0.56%) -0.01s (- 0.52%) 2.09s 2.13s
Bind Time 0.92s (± 1.17%) 0.92s (± 0.43%) +0.00s (+ 0.22%) 0.91s 0.93s
Check Time 5.44s (± 1.12%) 5.47s (± 0.99%) +0.03s (+ 0.53%) 5.27s 5.55s
Emit Time 6.23s (± 0.51%) 6.69s (± 0.99%) +0.46s (+ 7.30%) 6.61s 6.94s
Total Time 14.71s (± 0.27%) 15.18s (± 0.26%) +0.47s (+ 3.22%) 15.09s 15.26s
Monaco - node (v8.9.0, x64)
Memory used 325,474k (± 0.01%) 325,477k (± 0.02%) +3k (+ 0.00%) 325,362k 325,555k
Parse Time 1.54s (± 0.24%) 1.55s (± 0.29%) +0.01s (+ 0.45%) 1.54s 1.56s
Bind Time 0.89s (± 0.65%) 0.90s (± 0.85%) +0.00s (+ 0.22%) 0.88s 0.91s
Check Time 5.38s (± 0.45%) 5.37s (± 0.22%) -0.00s (- 0.07%) 5.34s 5.39s
Emit Time 3.49s (± 0.35%) 3.87s (± 0.42%) +0.38s (+10.89%) 3.83s 3.90s
Total Time 11.30s (± 0.28%) 11.69s (± 0.21%) +0.39s (+ 3.42%) 11.62s 11.73s
TFS - node (v8.9.0, x64)
Memory used 291,502k (± 0.02%) 291,572k (± 0.01%) +70k (+ 0.02%) 291,464k 291,641k
Parse Time 1.26s (± 0.46%) 1.26s (± 0.41%) -0.00s (- 0.08%) 1.25s 1.27s
Bind Time 0.74s (± 0.63%) 0.75s (± 0.69%) +0.01s (+ 1.22%) 0.74s 0.76s
Check Time 4.90s (± 0.81%) 5.01s (± 1.67%) +0.11s (+ 2.29%) 4.86s 5.15s
Emit Time 3.32s (± 2.31%) 3.74s (± 3.05%) +0.42s (+12.77%) 3.60s 3.99s
Total Time 10.22s (± 0.57%) 10.76s (± 0.41%) +0.54s (+ 5.29%) 10.65s 10.86s
material-ui - node (v8.9.0, x64)
Memory used 453,075k (± 0.01%) 452,802k (± 0.01%) -273k (- 0.06%) 452,726k 452,902k
Parse Time 2.11s (± 0.48%) 2.11s (± 0.45%) -0.00s (- 0.05%) 2.10s 2.14s
Bind Time 0.81s (± 1.30%) 0.81s (± 0.84%) -0.00s (- 0.12%) 0.79s 0.82s
Check Time 16.54s (± 0.59%) 16.46s (± 0.81%) -0.08s (- 0.49%) 16.07s 16.65s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 19.46s (± 0.48%) 19.38s (± 0.69%) -0.09s (- 0.44%) 18.98s 19.55s
Angular - node (v8.9.0, x86)
Memory used 185,609k (± 0.03%) 185,451k (± 0.02%) -158k (- 0.09%) 185,366k 185,541k
Parse Time 2.06s (± 0.68%) 2.05s (± 0.41%) -0.01s (- 0.53%) 2.03s 2.07s
Bind Time 1.07s (± 0.42%) 1.08s (± 0.70%) +0.01s (+ 0.75%) 1.06s 1.10s
Check Time 4.97s (± 0.48%) 4.98s (± 0.52%) +0.01s (+ 0.26%) 4.94s 5.04s
Emit Time 6.05s (± 1.11%) 6.41s (± 0.46%) +0.36s (+ 5.95%) 6.35s 6.47s
Total Time 14.15s (± 0.54%) 14.52s (± 0.31%) +0.37s (+ 2.63%) 14.45s 14.62s
Monaco - node (v8.9.0, x86)
Memory used 185,336k (± 0.02%) 185,342k (± 0.02%) +7k (+ 0.00%) 185,229k 185,432k
Parse Time 1.60s (± 0.59%) 1.60s (± 0.64%) -0.00s (- 0.06%) 1.58s 1.62s
Bind Time 0.77s (± 0.52%) 0.77s (± 0.84%) +0.00s (+ 0.39%) 0.76s 0.79s
Check Time 5.42s (± 0.47%) 5.37s (± 1.33%) -0.05s (- 0.94%) 5.10s 5.46s
Emit Time 2.85s (± 0.61%) 3.26s (± 2.91%) +0.41s (+14.53%) 3.17s 3.60s
Total Time 10.64s (± 0.27%) 11.00s (± 0.68%) +0.36s (+ 3.41%) 10.87s 11.24s
TFS - node (v8.9.0, x86)
Memory used 166,874k (± 0.02%) 166,987k (± 0.03%) +114k (+ 0.07%) 166,802k 167,058k
Parse Time 1.29s (± 0.47%) 1.29s (± 0.79%) +0.00s (+ 0.31%) 1.27s 1.32s
Bind Time 0.71s (± 0.56%) 0.72s (± 1.06%) +0.01s (+ 0.70%) 0.70s 0.74s
Check Time 4.65s (± 0.60%) 4.64s (± 0.53%) -0.00s (- 0.02%) 4.59s 4.69s
Emit Time 2.98s (± 0.72%) 3.50s (± 2.15%) +0.53s (+17.67%) 3.41s 3.78s
Total Time 9.62s (± 0.31%) 10.15s (± 0.77%) +0.53s (+ 5.55%) 10.03s 10.41s
material-ui - node (v8.9.0, x86)
Memory used 256,512k (± 0.02%) 256,335k (± 0.01%) -177k (- 0.07%) 256,277k 256,423k
Parse Time 2.17s (± 0.43%) 2.18s (± 0.50%) +0.01s (+ 0.46%) 2.16s 2.21s
Bind Time 0.68s (± 0.91%) 0.68s (± 0.87%) +0.00s (+ 0.59%) 0.68s 0.70s
Check Time 15.10s (± 0.70%) 15.25s (± 0.68%) +0.15s (+ 1.01%) 15.10s 15.53s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 17.95s (± 0.58%) 18.12s (± 0.61%) +0.17s (+ 0.93%) 17.94s 18.42s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-166-generic
Architecturex64
Available Memory16 GB
Available Memory1 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v8.9.0, x64)
  • node (v8.9.0, x86)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v8.9.0, x64)
  • Angular - node (v8.9.0, x86)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v8.9.0, x64)
  • Monaco - node (v8.9.0, x86)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v8.9.0, x64)
  • TFS - node (v8.9.0, x86)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v8.9.0, x64)
  • material-ui - node (v8.9.0, x86)
Benchmark Name Iterations
Current 37814 10
Baseline master 10

@DanielRosenwasser
Copy link
Member Author

DanielRosenwasser commented Apr 6, 2020 •
edited
Loading

It's really fascinating to see that this consistently adds up to half a second in compilation times on Node 10/12 for so many of these projects.

@andrewbranch
Copy link
Member

I just merged #37846 into this to see how it does.

@andrewbranch
Copy link
Member

Looks like all the semicolon-related ones are fixed, but I still see a bunch of synthesized statements starting immediately after a } token. https://github.com/microsoft/TypeScript/pull/37814/files#diff-c4678205d156e68a66bfc2a581356267R31

@andrewbranch
Copy link
Member

I’m not sure whether it’s possible to hit that in a refactor, but I could debug the JS emit on one of these and see what the logical error is. I just might not have a test case to add for it.

@andrewbranch
Copy link
Member

Pushed some commits that fix most of the problems at the cost of falling back to the non-source-peeking approach for any construct that fails to set an original node while going through the transformers. I fixed a couple instances of that in dd54136. Kind of interesting. I’m not sure any of these new commits would affect refactors at all.

@DanielRosenwasser DanielRosenwasser mentioned this pull request Apr 21, 2020
Closed
@mbroadst
Copy link

mbroadst commented May 3, 2020

@andrewbranch is this change slated for a particular version, or just an experiment?

@andrewbranch
Copy link
Member

It’s just an experiment, and in fact it’s more @DanielRosenwasser’s than mine, and I’m not sure what his thoughts on this are long-term. We’ve tossed around the idea of exposing this as an opt-in option. For my part, I’m currently pretty leery of that, because aside from the things that look clearly wrong in this diff, there are tons of constructs where we currently don’t check for whitespace, and adding a check to each of those places adds complexity and impacts performance.

@orta
Copy link
Contributor

orta commented Oct 21, 2020

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 21, 2020 •
edited
Loading

Heya @orta, I've started to run the tarball bundle task on this PR at aca7df3. You can monitor the build here.

@andrewbranch andrewbranch mentioned this pull request Jan 12, 2021
Closed
@eneko89 eneko89 mentioned this pull request Mar 11, 2021
Closed
@sandersn
Copy link
Member

To help with PR housekeeping, I'm going to close this draft PR. It's pretty old.

@sandersn sandersn closed this May 25, 2022
@DanielRosenwasser DanielRosenwasser mentioned this pull request Apr 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@DanielRosenwasser DanielRosenwasser

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Emit does not preserve empty lines
6 participants
@DanielRosenwasser @typescript-bot @andrewbranch @mbroadst @orta @sandersn