Skip to content

Emit source positions for open/close braces #47924

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
Merged
merged 4 commits into from
Sep 23, 2023
Merged

Emit source positions for open/close braces #47924

merged 4 commits into from
Sep 23, 2023

Conversation

rbuckton
Copy link
Contributor

This changes the emitter to emit source positions before and after open and close braces ({, }) so that tools like v8-to-istanbul can correctly map branches for code coverage. Brace source positions are ignored when the output is a declaration file.

NOTE: This does not emit source positions for all tokens that pass through emitTokenWithComment, as that significantly increases the size of the output .js.map file.

Fixes #39170

@typescript-bot typescript-bot added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Feb 16, 2022
@rbuckton
Copy link
Contributor Author

@typescript-bot perf test
@typescript-bot run dt
@typescript-bot test this
@typescript-bot user test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 17, 2022 •
edited
Loading

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

Update: The results are in!

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 17, 2022 •
edited
Loading

Heya @rbuckton, I've started to run the extended test suite on this PR at 07e8618. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 17, 2022 •
edited
Loading

Heya @rbuckton, I've started to run the parallelized Definitely Typed test suite on this PR at 07e8618. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Feb 17, 2022 •
edited
Loading

Heya @rbuckton, I've started to run the parallelized community code test suite on this PR at 07e8618. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

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

Here they are:

Comparison Report - main..47924

Metric main 47924 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 356,686k (± 0.02%) 356,739k (± 0.02%) +53k (+ 0.01%) 356,604k 356,922k
Parse Time 2.04s (± 0.28%) 2.04s (± 0.73%) -0.00s (- 0.10%) 2.01s 2.06s
Bind Time 0.86s (± 0.89%) 0.86s (± 0.35%) +0.00s (+ 0.35%) 0.85s 0.86s
Check Time 5.70s (± 0.32%) 5.69s (± 0.73%) -0.01s (- 0.11%) 5.59s 5.80s
Emit Time 5.92s (± 0.66%) 5.96s (± 0.62%) +0.03s (+ 0.59%) 5.87s 6.03s
Total Time 14.51s (± 0.34%) 14.54s (± 0.46%) +0.03s (+ 0.21%) 14.44s 14.73s
Compiler-Unions - node (v10.16.3, x64)
Memory used 205,502k (± 0.03%) 205,542k (± 0.02%) +40k (+ 0.02%) 205,438k 205,685k
Parse Time 0.83s (± 0.84%) 0.83s (± 0.80%) +0.00s (+ 0.24%) 0.82s 0.85s
Bind Time 0.53s (± 1.10%) 0.52s (± 1.06%) -0.01s (- 0.95%) 0.51s 0.53s
Check Time 7.79s (± 0.55%) 7.80s (± 0.65%) +0.01s (+ 0.17%) 7.72s 7.92s
Emit Time 2.51s (± 0.93%) 2.52s (± 0.79%) +0.01s (+ 0.56%) 2.47s 2.58s
Total Time 11.66s (± 0.46%) 11.68s (± 0.47%) +0.02s (+ 0.18%) 11.56s 11.80s
Monaco - node (v10.16.3, x64)
Memory used 343,249k (± 0.03%) 343,193k (± 0.02%) -56k (- 0.02%) 343,049k 343,418k
Parse Time 1.56s (± 0.45%) 1.56s (± 0.78%) -0.00s (- 0.19%) 1.54s 1.58s
Bind Time 0.76s (± 0.59%) 0.76s (± 0.59%) 0.00s ( 0.00%) 0.75s 0.77s
Check Time 5.56s (± 0.29%) 5.58s (± 0.50%) +0.02s (+ 0.38%) 5.52s 5.65s
Emit Time 3.27s (± 0.99%) 3.23s (± 1.06%) -0.04s (- 1.13%) 3.17s 3.32s
Total Time 11.15s (± 0.24%) 11.13s (± 0.33%) -0.01s (- 0.13%) 11.04s 11.23s
TFS - node (v10.16.3, x64)
Memory used 305,064k (± 0.02%) 305,048k (± 0.02%) -16k (- 0.01%) 304,906k 305,227k
Parse Time 1.27s (± 0.51%) 1.28s (± 0.60%) +0.01s (+ 0.55%) 1.26s 1.29s
Bind Time 0.72s (± 0.41%) 0.71s (± 0.70%) -0.00s (- 0.42%) 0.71s 0.73s
Check Time 5.15s (± 0.47%) 5.16s (± 0.68%) +0.00s (+ 0.08%) 5.10s 5.25s
Emit Time 3.40s (± 1.12%) 3.40s (± 1.60%) -0.00s (- 0.06%) 3.29s 3.51s
Total Time 10.54s (± 0.40%) 10.54s (± 0.59%) +0.00s (+ 0.05%) 10.38s 10.65s
material-ui - node (v10.16.3, x64)
Memory used 468,800k (± 0.01%) 468,828k (± 0.01%) +28k (+ 0.01%) 468,672k 468,990k
Parse Time 1.80s (± 0.34%) 1.80s (± 0.54%) -0.00s (- 0.11%) 1.78s 1.82s
Bind Time 0.67s (± 1.34%) 0.67s (± 1.02%) 0.00s ( 0.00%) 0.65s 0.68s
Check Time 14.02s (± 0.40%) 14.05s (± 0.58%) +0.03s (+ 0.19%) 13.87s 14.19s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.49s (± 0.34%) 16.52s (± 0.50%) +0.03s (+ 0.18%) 16.36s 16.66s
xstate - node (v10.16.3, x64)
Memory used 570,727k (± 0.02%) 570,716k (± 0.02%) -11k (- 0.00%) 570,387k 570,849k
Parse Time 2.58s (± 0.25%) 2.57s (± 0.52%) -0.01s (- 0.46%) 2.54s 2.60s
Bind Time 1.02s (± 0.69%) 1.02s (± 0.65%) -0.00s (- 0.10%) 1.01s 1.04s
Check Time 1.47s (± 0.55%) 1.48s (± 0.44%) +0.01s (+ 0.34%) 1.46s 1.49s
Emit Time 0.07s (± 3.14%) 0.07s (± 0.00%) -0.00s (- 1.41%) 0.07s 0.07s
Total Time 5.14s (± 0.27%) 5.14s (± 0.26%) -0.01s (- 0.12%) 5.11s 5.17s
Angular - node (v12.1.0, x64)
Memory used 334,326k (± 0.10%) 334,551k (± 0.02%) +225k (+ 0.07%) 334,351k 334,729k
Parse Time 2.03s (± 0.85%) 2.03s (± 0.47%) -0.00s (- 0.20%) 2.00s 2.05s
Bind Time 0.84s (± 0.87%) 0.84s (± 0.71%) -0.00s (- 0.24%) 0.83s 0.85s
Check Time 5.51s (± 0.77%) 5.48s (± 0.46%) -0.03s (- 0.54%) 5.42s 5.53s
Emit Time 6.16s (± 0.65%) 6.15s (± 0.68%) -0.01s (- 0.18%) 6.10s 6.28s
Total Time 14.54s (± 0.60%) 14.49s (± 0.41%) -0.05s (- 0.33%) 14.40s 14.69s
Compiler-Unions - node (v12.1.0, x64)
Memory used 193,000k (± 0.14%) 193,029k (± 0.07%) +29k (+ 0.01%) 192,490k 193,140k
Parse Time 0.83s (± 0.78%) 0.82s (± 0.41%) -0.01s (- 1.08%) 0.82s 0.83s
Bind Time 0.54s (± 1.28%) 0.53s (± 0.98%) -0.00s (- 0.75%) 0.52s 0.54s
Check Time 7.37s (± 1.00%) 7.32s (± 0.50%) -0.05s (- 0.68%) 7.27s 7.45s
Emit Time 2.50s (± 1.05%) 2.51s (± 0.98%) +0.01s (+ 0.40%) 2.46s 2.57s
Total Time 11.24s (± 0.75%) 11.18s (± 0.41%) -0.05s (- 0.49%) 11.07s 11.28s
Monaco - node (v12.1.0, x64)
Memory used 326,236k (± 0.02%) 326,186k (± 0.03%) -50k (- 0.02%) 326,042k 326,371k
Parse Time 1.54s (± 0.74%) 1.54s (± 0.72%) +0.00s (+ 0.13%) 1.52s 1.57s
Bind Time 0.74s (± 0.54%) 0.74s (± 0.51%) -0.00s (- 0.54%) 0.73s 0.74s
Check Time 5.48s (± 0.42%) 5.47s (± 0.47%) -0.01s (- 0.22%) 5.41s 5.53s
Emit Time 3.22s (± 0.27%) 3.24s (± 0.62%) +0.01s (+ 0.43%) 3.20s 3.29s
Total Time 10.98s (± 0.20%) 10.98s (± 0.40%) -0.00s (- 0.03%) 10.90s 11.08s
TFS - node (v12.1.0, x64)
Memory used 289,764k (± 0.02%) 289,776k (± 0.01%) +12k (+ 0.00%) 289,698k 289,861k
Parse Time 1.28s (± 0.68%) 1.28s (± 0.97%) +0.01s (+ 0.47%) 1.25s 1.31s
Bind Time 0.70s (± 1.38%) 0.71s (± 1.33%) +0.01s (+ 0.86%) 0.69s 0.72s
Check Time 5.07s (± 0.49%) 5.09s (± 0.56%) +0.02s (+ 0.49%) 5.01s 5.15s
Emit Time 3.43s (± 0.75%) 3.43s (± 0.48%) -0.00s (- 0.03%) 3.39s 3.45s
Total Time 10.47s (± 0.28%) 10.51s (± 0.35%) +0.04s (+ 0.34%) 10.40s 10.60s
material-ui - node (v12.1.0, x64)
Memory used 447,806k (± 0.06%) 447,844k (± 0.05%) +38k (+ 0.01%) 446,894k 448,007k
Parse Time 1.81s (± 0.59%) 1.81s (± 0.58%) +0.00s (+ 0.11%) 1.79s 1.83s
Bind Time 0.63s (± 0.54%) 0.64s (± 0.91%) +0.00s (+ 0.63%) 0.62s 0.65s
Check Time 12.67s (± 0.74%) 12.65s (± 0.62%) -0.02s (- 0.17%) 12.52s 12.82s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.11s (± 0.65%) 15.10s (± 0.55%) -0.01s (- 0.09%) 14.95s 15.27s
xstate - node (v12.1.0, x64)
Memory used 543,578k (± 1.90%) 536,678k (± 0.02%) -6,899k (- 1.27%) 536,499k 536,862k
Parse Time 2.53s (± 0.46%) 2.51s (± 0.42%) -0.02s (- 0.75%) 2.50s 2.55s
Bind Time 1.05s (± 0.53%) 1.05s (± 0.55%) -0.00s (- 0.48%) 1.04s 1.06s
Check Time 1.44s (± 0.80%) 1.44s (± 0.65%) -0.01s (- 0.42%) 1.42s 1.46s
Emit Time 0.07s (± 0.00%) 0.07s (± 0.00%) 0.00s ( 0.00%) 0.07s 0.07s
Total Time 5.10s (± 0.40%) 5.07s (± 0.35%) -0.03s (- 0.65%) 5.02s 5.12s
Angular - node (v14.15.1, x64)
Memory used 332,869k (± 0.01%) 332,855k (± 0.01%) -15k (- 0.00%) 332,809k 332,905k
Parse Time 2.01s (± 0.57%) 2.02s (± 0.72%) +0.00s (+ 0.25%) 2.00s 2.06s
Bind Time 0.89s (± 0.50%) 0.89s (± 0.58%) -0.00s (- 0.11%) 0.88s 0.90s
Check Time 5.51s (± 0.46%) 5.56s (± 0.53%) +0.05s (+ 0.96%) 5.51s 5.62s
Emit Time 6.21s (± 0.60%) 6.29s (± 0.76%) +0.09s (+ 1.37%) 6.18s 6.39s
Total Time 14.62s (± 0.35%) 14.76s (± 0.39%) +0.14s (+ 0.99%) 14.59s 14.84s
Compiler-Unions - node (v14.15.1, x64)
Memory used 194,012k (± 0.57%) 193,417k (± 0.64%) -596k (- 0.31%) 191,708k 195,134k
Parse Time 0.85s (± 0.68%) 0.85s (± 0.61%) +0.00s (+ 0.24%) 0.84s 0.86s
Bind Time 0.57s (± 0.98%) 0.56s (± 0.87%) -0.00s (- 0.70%) 0.56s 0.58s
Check Time 7.41s (± 0.74%) 7.38s (± 0.41%) -0.03s (- 0.38%) 7.32s 7.48s
Emit Time 2.48s (± 1.36%) 2.48s (± 0.70%) -0.00s (- 0.16%) 2.45s 2.52s
Total Time 11.31s (± 0.46%) 11.28s (± 0.31%) -0.04s (- 0.31%) 11.20s 11.37s
Monaco - node (v14.15.1, x64)
Memory used 325,011k (± 0.00%) 325,016k (± 0.01%) +5k (+ 0.00%) 324,978k 325,059k
Parse Time 1.56s (± 0.68%) 1.56s (± 0.80%) +0.01s (+ 0.45%) 1.54s 1.60s
Bind Time 0.77s (± 0.61%) 0.77s (± 0.80%) -0.00s (- 0.13%) 0.76s 0.78s
Check Time 5.39s (± 0.47%) 5.39s (± 0.33%) +0.00s (+ 0.04%) 5.36s 5.43s
Emit Time 3.27s (± 0.65%) 3.29s (± 0.70%) +0.02s (+ 0.67%) 3.25s 3.35s
Total Time 10.98s (± 0.38%) 11.01s (± 0.30%) +0.03s (+ 0.26%) 10.95s 11.10s
TFS - node (v14.15.1, x64)
Memory used 288,583k (± 0.01%) 288,575k (± 0.01%) -8k (- 0.00%) 288,534k 288,611k
Parse Time 1.31s (± 1.60%) 1.30s (± 1.65%) -0.00s (- 0.31%) 1.27s 1.35s
Bind Time 0.73s (± 1.24%) 0.74s (± 0.81%) +0.00s (+ 0.55%) 0.72s 0.75s
Check Time 5.06s (± 0.54%) 5.06s (± 0.44%) -0.00s (- 0.04%) 5.02s 5.12s
Emit Time 3.51s (± 1.81%) 3.52s (± 1.12%) +0.01s (+ 0.37%) 3.37s 3.57s
Total Time 10.61s (± 0.73%) 10.62s (± 0.32%) +0.01s (+ 0.08%) 10.56s 10.70s
material-ui - node (v14.15.1, x64)
Memory used 446,057k (± 0.06%) 446,057k (± 0.06%) +0k (+ 0.00%) 444,890k 446,222k
Parse Time 1.85s (± 0.65%) 1.84s (± 0.32%) -0.01s (- 0.59%) 1.83s 1.86s
Bind Time 0.69s (± 0.49%) 0.69s (± 0.58%) -0.00s (- 0.58%) 0.68s 0.70s
Check Time 12.79s (± 0.71%) 12.82s (± 0.88%) +0.03s (+ 0.22%) 12.61s 13.06s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.34s (± 0.62%) 15.35s (± 0.76%) +0.01s (+ 0.08%) 15.13s 15.61s
xstate - node (v14.15.1, x64)
Memory used 534,460k (± 0.01%) 534,436k (± 0.01%) -24k (- 0.00%) 534,383k 534,508k
Parse Time 2.57s (± 0.39%) 2.57s (± 0.25%) -0.01s (- 0.19%) 2.56s 2.58s
Bind Time 1.15s (± 0.65%) 1.16s (± 0.91%) +0.00s (+ 0.35%) 1.13s 1.18s
Check Time 1.47s (± 0.64%) 1.47s (± 0.30%) -0.00s (- 0.07%) 1.46s 1.48s
Emit Time 0.07s (± 4.13%) 0.07s (± 0.00%) -0.00s (- 2.78%) 0.07s 0.07s
Total Time 5.27s (± 0.33%) 5.28s (± 0.30%) +0.00s (+ 0.06%) 5.24s 5.31s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory5 GB
CPUs4 × Intel(R) Core(TM) i7-4770 CPU @ 3.40GHz
Hosts
  • node (v10.16.3, x64)
  • node (v12.1.0, x64)
  • node (v14.15.1, x64)
Scenarios
  • Angular - node (v10.16.3, x64)
  • Angular - node (v12.1.0, x64)
  • Angular - node (v14.15.1, x64)
  • Compiler-Unions - node (v10.16.3, x64)
  • Compiler-Unions - node (v12.1.0, x64)
  • Compiler-Unions - node (v14.15.1, x64)
  • Monaco - node (v10.16.3, x64)
  • Monaco - node (v12.1.0, x64)
  • Monaco - node (v14.15.1, x64)
  • TFS - node (v10.16.3, x64)
  • TFS - node (v12.1.0, x64)
  • TFS - node (v14.15.1, x64)
  • material-ui - node (v10.16.3, x64)
  • material-ui - node (v12.1.0, x64)
  • material-ui - node (v14.15.1, x64)
  • xstate - node (v10.16.3, x64)
  • xstate - node (v12.1.0, x64)
  • xstate - node (v14.15.1, x64)
Benchmark Name Iterations
Current 47924 10
Baseline main 10

Developer Information:

Download Benchmark

@typescript-bot typescript-bot mentioned this pull request Feb 17, 2022
Open
@typescript-bot
Copy link
Collaborator

The user suite test run you requested has finished and failed. I've opened a PR with the baseline diff from master.

andrewbranch
andrewbranch reviewed Feb 18, 2022
View reviewed changes
src/compiler/emitter.ts Outdated
inlineSourceMap: compilerOptions.inlineSourceMap,
extendedDiagnostics: compilerOptions.extendedDiagnostics,
onlyPrintJsDocStyle: true,
omitBraceSourcePositions: true,
Copy link
Member

Choose a reason for hiding this comment

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

Can we make this name more obviously about source maps? I think most people dealing with the emitter API have .js/.d.ts emit in their minds, where this name makes no sense.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I can change the name, though this property is marked /*@internal*/ so it shouldn't impact API consumers outside of our own team.

@sandersn
Copy link
Member

@rbuckton I'm going through all the ready-to-merge PRs, and this one is all signed off. Is it worth bringing up to date and merging it for 5.1?

@sandersn
Copy link
Member

@rbuckton Same question as before, but for 5.3 -- should we take this one or close it?

@rbuckton
Copy link
Contributor Author

I'll see if I can get it up to date.

@rbuckton rbuckton merged commit 956a363 into main Sep 23, 2023
@rbuckton rbuckton deleted the braceSourceMaps branch September 23, 2023 06:41
snovader pushed a commit to EG-A-S/TypeScript that referenced this pull request Sep 23, 2023
@rbuckton @snovader
Emit source positions for open/close braces (microsoft#47924)
b6f99ab
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@sandersn sandersn sandersn approved these changes

@andrewbranch andrewbranch andrewbranch approved these changes

+1 more reviewer

@raghavthind2005 raghavthind2005 raghavthind2005 approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@rbuckton rbuckton

Labels

Author: Team For Milestone Bug PRs that fix a bug with a specific milestone

Projects

PR Backlog
Archived in project

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

sourcemap mapping is generated before node generates newline and indent

5 participants

@rbuckton @typescript-bot @sandersn @andrewbranch @raghavthind2005