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(linter): parse ts-directive manually #845

Merged
merged 1 commit into from
Sep 3, 2023

Conversation

Devin-Yeung
Copy link
Contributor

parse ts directive manually instead of using regex, mentioned in #741 (comment). I roughly check the benchmark results on vscode repo, seems that the performance changes is very subtle, do we need to test on other repos?

[Run on main branch]
Total: 406.88ms
   Time |     % | Rule
 406.88 | 100.0% | ban-ts-comment

Finished in 2235ms on 4508 files with 1 rules using 2 threads.
Found 46 warnings and 0 errors.

[Run on PR branch]
Rule timings in milliseconds:
Total: 404.24ms
   Time |     % | Rule
 404.24 | 100.0% | ban-ts-comment

Finished in 2223ms on 4508 files with 1 rules using 2 threads.
Found 46 warnings and 0 errors.

@github-actions github-actions bot added the A-linter Area - Linter label Sep 3, 2023
@codspeed-hq
Copy link

codspeed-hq bot commented Sep 3, 2023

CodSpeed Performance Report

Merging #845 will not alter performance

Comparing Devin-Yeung:perf-ban-ts-comment (b6a6fdc) with main (73be3ea)

Summary

✅ 17 untouched benchmarks

Copy link
Member

@Boshen Boshen left a comment

Choose a reason for hiding this comment

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

Thank you! These small numbers add up, every optimization counts.

@Boshen Boshen merged commit a969f69 into oxc-project:main Sep 3, 2023
16 checks passed
@Devin-Yeung Devin-Yeung deleted the perf-ban-ts-comment branch September 3, 2023 11:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-linter Area - Linter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants