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

ci: run all linter benchmarks #3222

Closed
wants to merge 1 commit into from

Conversation

overlookmotel
Copy link
Collaborator

@overlookmotel overlookmotel commented May 10, 2024

After #3221 we can re-enable the linter benchmarks which were disabled in #3172, as they won't hog so much CI time any more.

Copy link
Collaborator Author

overlookmotel commented May 10, 2024

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @overlookmotel and the rest of your teammates on Graphite Graphite

@overlookmotel overlookmotel marked this pull request as ready for review May 10, 2024 10:17
Copy link

codspeed-hq bot commented May 10, 2024

CodSpeed Performance Report

Merging #3222 will not alter performance

Comparing 05-10-ci_run_all_linter_benchmarks (31a02a5) with main (5e36e0d)

Summary

✅ 27 untouched benchmarks

🆕 3 new benchmarks

Benchmarks breakdown

Benchmark main 05-10-ci_run_all_linter_benchmarks Change
🆕 linter[RadixUIAdoptionSection.jsx] N/A 7.3 ms N/A
🆕 linter[antd.js] N/A 10.4 s N/A
🆕 linter[pdf.mjs] N/A 1.8 s N/A

@overlookmotel
Copy link
Collaborator Author

overlookmotel commented May 10, 2024

I still don't understand why the linter benchmark takes so long to build. The linter itself is an enormous bunch of code, so you'd expect it, but the benchmark also takes ages to build.

--timings output on CI:

linter-bench-ci

Base automatically changed from 05-10-perf_bench_build_each_benchmark_only_with_deps_it_needs to main May 10, 2024 14:01
@Boshen
Copy link
Member

Boshen commented May 10, 2024

Github action queues the jobs, adding too many linter benchmarks will drag out these jobs. Let me put this on hold.

Let me fix the generic monomorphization problem first, I'm getting there.

@Boshen Boshen marked this pull request as draft May 10, 2024 14:05
@overlookmotel overlookmotel force-pushed the 05-10-ci_run_all_linter_benchmarks branch from bce02fe to 31a02a5 Compare May 10, 2024 14:21
@overlookmotel
Copy link
Collaborator Author

OK, no problem. Actually we should probably drop the 5th benchmark (antd.js) anyway as it takes too long to run even all by itself.

My logic was that now that the jobs don't include building the benchmark and run in ~1 minute, they don't gum up the queue for long, so contention is less of a concern.

@Boshen
Copy link
Member

Boshen commented May 14, 2024

Feels like 2 linter benchmarks is enough for me, closing.

@Boshen Boshen closed this May 14, 2024
@Boshen Boshen deleted the 05-10-ci_run_all_linter_benchmarks branch May 14, 2024 04:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants