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

Draft: fix benchmarks #37

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

nilptr
Copy link

@nilptr nilptr commented Dec 18, 2024

Currently, all runs in each benchmark test use the same resolver instance, which has a built-in cache. As a result, except for the first run, all subsequent runs hit the cache and execute significantly faster. This causes the benchmark results to fail to accurately reflect its performance.

group.bench_with_input(BenchmarkId::from_parameter("single-thread"), &data, |b, data| {
let oxc_resolver = oxc_resolver();
b.iter(|| {
for (path, request) in data {
_ = oxc_resolver.resolve(path, request);
}
});
});

Here are the benchmark results from the main branch:

main

If we create a new resolver instance in each run, the benchmark results will look like this:

bench-without-cache

(branch: nilptr/fix/bench-without-cache)

Since we want to compare the performance with async fs version, I feel it's better to fix the benchmarks first.

Also, as we need to use Tokio as the async runtime, it would be better to start switching to Tokio as the multi-thread runner. This will make it easier for us to make comparisons.

Finally, I added a multi-threaded version of the "resolve from symlinks" benchmark case, which better demonstrates the potential of async fs.

Here are the benchmark results from the source branch:

bench-without-cache-tokio

The multi-thread benchmark case takes twice as long, while the others remain mostly unchanged. Probably because Tokio's scheduling strategy is not as efficient as Rayon’s.

Copy link

codspeed-hq bot commented Dec 19, 2024

CodSpeed Performance Report

Merging #37 will degrade performances by 51.09%

Comparing nilptr:nilptr/fix/bench-without-cache-tokio (82eab8b) with main (e3d5975)

Summary

❌ 2 regressions
🆕 2 new benchmarks
⁉️ 1 dropped benchmarks

⚠️ Please fix the performance issues or acknowledge them on CodSpeed.

Benchmarks breakdown

Benchmark main nilptr:nilptr/fix/bench-without-cache-tokio Change
resolver[multi-thread] 501.6 µs 1,025.7 µs -51.09%
⁉️ resolver[resolve from symlinks] 71.6 ms N/A N/A
🆕 resolver[resolve-from-symlinks-multi-thread] N/A 159.3 ms N/A
🆕 resolver[resolve-from-symlinks] N/A 133.4 ms N/A
resolver[single-thread] 467.1 µs 918.1 µs -49.13%

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.

1 participant