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(url): minimize String::push calls in parse_scheme #789

Closed
wants to merge 2 commits into from

Conversation

littledivy
Copy link
Contributor

Minimize calls to String::push into a single allocation after encoutering : in parse_scheme.

~1.5x improvement in Url::parse("http://example.com")?

main:

time 353 ms rate 2831289
time 353 ms rate 2830078
time 354 ms rate 2821986
time 353 ms rate 2829860
time 353 ms rate 2829671
time 354 ms rate 2821004
time 353 ms rate 2831683
time 353 ms rate 2830756
time 353 ms rate 2830787
time 352 ms rate 2834233

This patch:

time 308 ms rate 3245196
time 308 ms rate 3240815
time 306 ms rate 3261353
time 306 ms rate 3259889
time 309 ms rate 3227921
time 306 ms rate 3260827
time 306 ms rate 3262840
time 309 ms rate 3235442
time 308 ms rate 3239239
time 306 ms rate 3264659

This patch + @AaronO's #761:

time 206 ms rate 4833188
time 206 ms rate 4854101
time 206 ms rate 4853265
time 206 ms rate 4838242
time 211 ms rate 4721031
time 206 ms rate 4842378
time 205 ms rate 4857185
time 205 ms rate 4855094
time 206 ms rate 4841436
Benchmark code
const COUNT: usize = 1000000;
for _ in 0..10 {
    let start = std::time::Instant::now();
    for _ in 0..COUNT {
      let url = url::Url::parse("http://www.example.com/").unwrap();
    }
    let elasped = start.elapsed();
    let rate = COUNT as f64 / elasped.as_secs_f64();
    println!("time {} ms rate {}", elasped.as_millis(), rate.round());
}

@littledivy littledivy closed this Sep 6, 2022
@littledivy
Copy link
Contributor Author

I was benchmarking against the crates.io version, there is no visible improvement from main

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

1 participant