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

perf(ts-estree): don't create Program in parse() #148

Merged
merged 4 commits into from
Jan 29, 2019
Merged

Conversation

JamesHenry
Copy link
Member

@JamesHenry JamesHenry commented Jan 27, 2019

Improves the performance for the parse() use case - e.g. Prettier's current usage.

From the existing benchmark it seems to be ~50% speed increase gained by not bothering with a full ts.Program and just creating a ts.SourceFile directly.

@typescript-eslint/benchmark: original-parse x 7,332 ops/sec ±6.34% (74 runs sampled)
@typescript-eslint/benchmark: updated-parse x 11,401 ops/sec ±1.54% (87 runs sampled)

I also streamlined some things and added more inline documentation

@codecov
Copy link

codecov bot commented Jan 27, 2019

Codecov Report

Merging #148 into master will increase coverage by 0.38%.
The diff coverage is 96.61%.

@@            Coverage Diff             @@
##           master     #148      +/-   ##
==========================================
+ Coverage   95.31%   95.69%   +0.38%     
==========================================
  Files          49       49              
  Lines        2477     2487      +10     
  Branches      370      373       +3     
==========================================
+ Hits         2361     2380      +19     
+ Misses         69       63       -6     
+ Partials       47       44       -3
Impacted Files Coverage Δ
packages/typescript-estree/src/parser.ts 91.3% <96.61%> (+9.39%) ⬆️

@JamesHenry
Copy link
Member Author

JamesHenry commented Jan 27, 2019

@uniqueiniquity FYI after I made these gains with the parse() use-case I tried to keep going and improve the parseAndGenerateServices() that we use in linting, but I'm afraid I didn't make any significant progress yet

@uniqueiniquity
Copy link
Contributor

Makes sense to me. For syntactic stuff I don’t see any reason to have the ts.Program.

@armano2 armano2 added the package: typescript-estree Issues related to @typescript-eslint/typescript-estree label Jan 27, 2019
@JamesHenry JamesHenry merged commit aacf5b0 into master Jan 29, 2019
@JamesHenry JamesHenry deleted the parse-perf branch January 29, 2019 02:25
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 21, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
package: typescript-estree Issues related to @typescript-eslint/typescript-estree
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants