Skip to content

Emit function name when emitting arrow functions in ES5 #46042

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
Closed
wants to merge 7 commits into from
Closed

Emit function name when emitting arrow functions in ES5 #46042

wants to merge 7 commits into from

Conversation

agentcooper
Copy link
Contributor

Fixes #6433.

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Sep 24, 2021
agentcooper
agentcooper commented Sep 24, 2021
View reviewed changes
src/compiler/transformers/es2015.ts Outdated
@@ -4417,4 +4421,40 @@ namespace ts {
return isIdentifier(expression) && expression.escapedText === "arguments";
}
}

function forEachFreeIdentifier(node: Node, cb: (id: Identifier) => void): void {
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've copied this one from src/services/codefixes/convertToEs6Module.ts. What will be the best place to put it so it can be shared?

Copy link
Member

Choose a reason for hiding this comment

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

src/compiler/utilities.ts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@andrewbranch thanks, done

@DanielRosenwasser
Copy link
Member

A little concerned about the tree walk, but let's see what the perf suite shows.

@typescript-bot perf test this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Sep 24, 2021 •
edited
Loading

Heya @DanielRosenwasser, I've started to run the perf test suite on this PR at 34f6e9d. 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 - main..46042

Metric main 46042 Delta Best Worst
Angular - node (v10.16.3, x64)
Memory used 352,323k (± 0.02%) 352,390k (± 0.02%) +67k (+ 0.02%) 352,236k 352,603k
Parse Time 1.90s (± 0.36%) 1.91s (± 0.87%) +0.01s (+ 0.53%) 1.89s 1.96s
Bind Time 0.85s (± 0.80%) 0.85s (± 0.90%) -0.00s (- 0.00%) 0.83s 0.87s
Check Time 5.44s (± 0.37%) 5.44s (± 0.26%) -0.00s (- 0.06%) 5.40s 5.46s
Emit Time 5.84s (± 0.71%) 5.82s (± 0.46%) -0.03s (- 0.46%) 5.77s 5.90s
Total Time 14.04s (± 0.30%) 14.02s (± 0.27%) -0.02s (- 0.15%) 13.94s 14.10s
Compiler-Unions - node (v10.16.3, x64)
Memory used 203,736k (± 0.03%) 203,705k (± 0.03%) -31k (- 0.02%) 203,554k 203,826k
Parse Time 0.79s (± 0.73%) 0.79s (± 0.63%) -0.00s (- 0.51%) 0.78s 0.80s
Bind Time 0.51s (± 0.96%) 0.52s (± 0.94%) +0.01s (+ 1.95%) 0.51s 0.53s
Check Time 7.89s (± 0.52%) 7.87s (± 0.56%) -0.02s (- 0.19%) 7.78s 7.98s
Emit Time 2.44s (± 0.87%) 2.46s (± 0.80%) +0.02s (+ 0.70%) 2.40s 2.49s
Total Time 11.63s (± 0.50%) 11.64s (± 0.45%) +0.00s (+ 0.02%) 11.54s 11.78s
Monaco - node (v10.16.3, x64)
Memory used 340,785k (± 0.02%) 340,737k (± 0.02%) -48k (- 0.01%) 340,571k 340,854k
Parse Time 1.45s (± 0.56%) 1.45s (± 0.74%) -0.00s (- 0.28%) 1.41s 1.46s
Bind Time 0.75s (± 0.86%) 0.75s (± 0.87%) -0.00s (- 0.53%) 0.74s 0.76s
Check Time 5.44s (± 0.47%) 5.45s (± 0.65%) +0.01s (+ 0.13%) 5.40s 5.58s
Emit Time 3.16s (± 1.02%) 3.14s (± 0.55%) -0.02s (- 0.70%) 3.11s 3.17s
Total Time 10.81s (± 0.30%) 10.78s (± 0.36%) -0.02s (- 0.21%) 10.72s 10.92s
TFS - node (v10.16.3, x64)
Memory used 304,195k (± 0.01%) 304,155k (± 0.02%) -40k (- 0.01%) 304,042k 304,322k
Parse Time 1.18s (± 0.31%) 1.18s (± 0.49%) +0.00s (+ 0.09%) 1.16s 1.19s
Bind Time 0.71s (± 0.67%) 0.71s (± 1.05%) +0.00s (+ 0.42%) 0.69s 0.72s
Check Time 4.93s (± 0.52%) 4.96s (± 0.42%) +0.03s (+ 0.63%) 4.92s 5.02s
Emit Time 3.31s (± 0.68%) 3.32s (± 1.34%) +0.01s (+ 0.30%) 3.23s 3.43s
Total Time 10.12s (± 0.38%) 10.17s (± 0.40%) +0.04s (+ 0.43%) 10.06s 10.26s
material-ui - node (v10.16.3, x64)
Memory used 470,430k (± 0.01%) 470,462k (± 0.01%) +33k (+ 0.01%) 470,373k 470,589k
Parse Time 1.73s (± 0.52%) 1.73s (± 0.69%) +0.00s (+ 0.12%) 1.71s 1.76s
Bind Time 0.67s (± 0.55%) 0.66s (± 0.55%) -0.00s (- 0.30%) 0.66s 0.67s
Check Time 14.34s (± 0.54%) 14.39s (± 0.61%) +0.04s (+ 0.30%) 14.26s 14.69s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 16.73s (± 0.46%) 16.78s (± 0.52%) +0.05s (+ 0.28%) 16.65s 17.08s
Angular - node (v12.1.0, x64)
Memory used 330,097k (± 0.09%) 330,290k (± 0.03%) +194k (+ 0.06%) 330,128k 330,507k
Parse Time 1.88s (± 0.59%) 1.89s (± 0.62%) +0.01s (+ 0.80%) 1.87s 1.92s
Bind Time 0.83s (± 0.69%) 0.84s (± 1.15%) +0.01s (+ 1.33%) 0.83s 0.87s
Check Time 5.29s (± 0.82%) 5.29s (± 0.69%) +0.00s (+ 0.02%) 5.21s 5.38s
Emit Time 6.10s (± 0.83%) 6.09s (± 0.84%) -0.00s (- 0.07%) 5.98s 6.21s
Total Time 14.09s (± 0.60%) 14.11s (± 0.64%) +0.02s (+ 0.12%) 13.90s 14.29s
Compiler-Unions - node (v12.1.0, x64)
Memory used 191,189k (± 0.10%) 191,153k (± 0.11%) -36k (- 0.02%) 190,605k 191,445k
Parse Time 0.78s (± 0.63%) 0.78s (± 0.71%) +0.00s (+ 0.26%) 0.77s 0.79s
Bind Time 0.53s (± 1.04%) 0.54s (± 0.69%) +0.00s (+ 0.56%) 0.53s 0.54s
Check Time 7.40s (± 0.79%) 7.38s (± 0.66%) -0.02s (- 0.23%) 7.27s 7.49s
Emit Time 2.44s (± 0.43%) 2.47s (± 0.95%) +0.02s (+ 0.98%) 2.42s 2.54s
Total Time 11.15s (± 0.52%) 11.17s (± 0.44%) +0.01s (+ 0.11%) 11.05s 11.28s
Monaco - node (v12.1.0, x64)
Memory used 323,859k (± 0.03%) 323,882k (± 0.02%) +23k (+ 0.01%) 323,745k 323,975k
Parse Time 1.43s (± 0.77%) 1.43s (± 0.51%) +0.00s (+ 0.35%) 1.42s 1.45s
Bind Time 0.73s (± 0.68%) 0.73s (± 0.68%) -0.00s (- 0.00%) 0.72s 0.74s
Check Time 5.32s (± 0.31%) 5.31s (± 0.66%) -0.01s (- 0.19%) 5.23s 5.39s
Emit Time 3.20s (± 0.67%) 3.20s (± 0.57%) -0.00s (- 0.12%) 3.14s 3.23s
Total Time 10.68s (± 0.25%) 10.66s (± 0.33%) -0.01s (- 0.10%) 10.60s 10.75s
TFS - node (v12.1.0, x64)
Memory used 288,931k (± 0.01%) 288,939k (± 0.02%) +8k (+ 0.00%) 288,782k 289,064k
Parse Time 1.20s (± 0.63%) 1.20s (± 0.89%) -0.00s (- 0.33%) 1.18s 1.23s
Bind Time 0.70s (± 1.27%) 0.70s (± 0.85%) +0.01s (+ 1.15%) 0.69s 0.72s
Check Time 4.88s (± 0.38%) 4.89s (± 0.42%) +0.02s (+ 0.39%) 4.85s 4.93s
Emit Time 3.36s (± 0.98%) 3.39s (± 0.78%) +0.03s (+ 0.77%) 3.32s 3.45s
Total Time 10.14s (± 0.44%) 10.19s (± 0.44%) +0.05s (+ 0.48%) 10.06s 10.27s
material-ui - node (v12.1.0, x64)
Memory used 449,184k (± 0.02%) 449,086k (± 0.06%) -97k (- 0.02%) 448,070k 449,388k
Parse Time 1.73s (± 0.52%) 1.72s (± 0.39%) -0.01s (- 0.29%) 1.71s 1.74s
Bind Time 0.64s (± 0.81%) 0.64s (± 1.14%) -0.00s (- 0.47%) 0.62s 0.65s
Check Time 12.92s (± 0.65%) 12.93s (± 0.43%) +0.01s (+ 0.04%) 12.81s 13.06s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.29s (± 0.55%) 15.29s (± 0.42%) -0.00s (- 0.03%) 15.14s 15.45s
Angular - node (v14.15.1, x64)
Memory used 328,746k (± 0.01%) 328,759k (± 0.01%) +13k (+ 0.00%) 328,701k 328,818k
Parse Time 1.90s (± 0.29%) 1.91s (± 0.61%) +0.01s (+ 0.42%) 1.89s 1.94s
Bind Time 0.88s (± 0.53%) 0.88s (± 1.33%) +0.00s (+ 0.11%) 0.86s 0.92s
Check Time 5.33s (± 0.43%) 5.34s (± 0.41%) +0.01s (+ 0.23%) 5.30s 5.38s
Emit Time 6.21s (± 1.02%) 6.18s (± 0.81%) -0.03s (- 0.45%) 6.09s 6.29s
Total Time 14.32s (± 0.41%) 14.32s (± 0.43%) -0.01s (- 0.05%) 14.15s 14.43s
Compiler-Unions - node (v14.15.1, x64)
Memory used 192,673k (± 0.36%) 192,358k (± 0.49%) -315k (- 0.16%) 189,813k 193,028k
Parse Time 0.81s (± 0.42%) 0.81s (± 0.59%) 0.00s ( 0.00%) 0.80s 0.82s
Bind Time 0.56s (± 0.89%) 0.55s (± 0.54%) -0.00s (- 0.72%) 0.55s 0.56s
Check Time 7.55s (± 0.71%) 7.55s (± 0.90%) +0.00s (+ 0.05%) 7.42s 7.69s
Emit Time 2.44s (± 1.14%) 2.44s (± 1.12%) +0.00s (+ 0.08%) 2.39s 2.52s
Total Time 11.35s (± 0.61%) 11.36s (± 0.74%) +0.01s (+ 0.04%) 11.19s 11.56s
Monaco - node (v14.15.1, x64)
Memory used 322,698k (± 0.00%) 322,712k (± 0.00%) +14k (+ 0.00%) 322,692k 322,732k
Parse Time 1.48s (± 0.50%) 1.48s (± 0.74%) 0.00s ( 0.00%) 1.47s 1.51s
Bind Time 0.76s (± 0.79%) 0.76s (± 0.77%) +0.00s (+ 0.26%) 0.74s 0.77s
Check Time 5.26s (± 0.44%) 5.27s (± 0.56%) +0.01s (+ 0.23%) 5.23s 5.34s
Emit Time 3.23s (± 0.90%) 3.23s (± 0.64%) -0.00s (- 0.09%) 3.17s 3.26s
Total Time 10.73s (± 0.39%) 10.74s (± 0.39%) +0.01s (+ 0.07%) 10.65s 10.82s
TFS - node (v14.15.1, x64)
Memory used 287,874k (± 0.01%) 287,864k (± 0.01%) -10k (- 0.00%) 287,814k 287,895k
Parse Time 1.21s (± 0.48%) 1.23s (± 2.03%) +0.02s (+ 1.40%) 1.20s 1.32s
Bind Time 0.79s (± 4.31%) 0.79s (± 4.33%) +0.01s (+ 0.63%) 0.71s 0.83s
Check Time 4.87s (± 0.61%) 4.89s (± 0.46%) +0.02s (+ 0.41%) 4.83s 4.93s
Emit Time 3.45s (± 0.72%) 3.46s (± 0.51%) +0.02s (+ 0.49%) 3.42s 3.50s
Total Time 10.32s (± 0.44%) 10.37s (± 0.39%) +0.06s (+ 0.54%) 10.28s 10.46s
material-ui - node (v14.15.1, x64)
Memory used 447,476k (± 0.06%) 447,578k (± 0.01%) +102k (+ 0.02%) 447,537k 447,644k
Parse Time 1.77s (± 0.43%) 1.77s (± 0.43%) -0.01s (- 0.45%) 1.75s 1.78s
Bind Time 0.69s (± 0.43%) 0.70s (± 0.68%) +0.01s (+ 1.59%) 0.69s 0.71s
Check Time 13.11s (± 0.74%) 13.09s (± 0.74%) -0.02s (- 0.18%) 12.95s 13.35s
Emit Time 0.00s (± 0.00%) 0.00s (± 0.00%) 0.00s ( NaN%) 0.00s 0.00s
Total Time 15.58s (± 0.64%) 15.56s (± 0.58%) -0.02s (- 0.13%) 15.43s 15.81s
System
Machine Namets-ci-ubuntu
Platformlinux 4.4.0-210-generic
Architecturex64
Available Memory16 GB
Available Memory9 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)
Benchmark Name Iterations
Current 46042 10
Baseline main 10

Developer Information:

Download Benchmark

@agentcooper
Copy link
Contributor Author

@DanielRosenwasser It seems that the perf results look fine. Could you please check my comment above about the placement of utility functions?

@agentcooper
Copy link
Contributor Author

@DanielRosenwasser @andrewbranch @sandersn Any chance this will make it to the 4.6?

@sandersn
Copy link
Member

sandersn commented Dec 3, 2021

The bug for this change is old and, unfortunately, es5 is still the default target for Typescript, so this would change emit all over the world for very little benefit, as laid out in @rbuckton's comment on the bug.

We should not take this PR or fix the bug.

@sandersn sandersn closed this Dec 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewbranch andrewbranch andrewbranch left review comments

Assignees

@rbuckton rbuckton

Labels
For Backlog Bug PRs that fix a backlog bug
Projects
PR Backlog
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Naming arrow functions
6 participants
@agentcooper @DanielRosenwasser @typescript-bot @sandersn @andrewbranch @rbuckton