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

Use a clone to allocate BytesStart #46

Closed
wants to merge 1 commit into from

Conversation

meven
Copy link
Contributor

@meven meven commented Feb 14, 2019

Was inspired by #41
Using a clone to allocate BytesStart allows to save calling borrowed_name in the loop resulting in some performance improvements:

==>  ./flamegraph/test/perf-iperf-stacks-pidtid-01.txt  <==
Benchmark #1: target/release/inferno-collapse-perf --all ./flamegraph/test/perf-iperf-stacks-pidtid-01.txt
  Time (mean ± σ):      11.3 ms ±   3.1 ms    [User: 9.0 ms, System: 2.4 ms]
  Range (min … max):     4.3 ms …  19.1 ms    189 runs
 
Benchmark #2: target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-iperf-stacks-pidtid-01.txt
  Time (mean ± σ):       9.0 ms ±   5.0 ms    [User: 7.1 ms, System: 2.0 ms]
  Range (min … max):     3.5 ms …  19.3 ms    172 runs
 
Summary
  'target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-iperf-stacks-pidtid-01.txt' ran
    1.26 ± 0.79 times faster than 'target/release/inferno-collapse-perf --all ./flamegraph/test/perf-iperf-stacks-pidtid-01.txt'


==>  ./flamegraph/test/perf-java-stacks-01.txt  <==
Benchmark #1: target/release/inferno-collapse-perf --all ./flamegraph/test/perf-java-stacks-01.txt
  Time (mean ± σ):       7.9 ms ±   1.5 ms    [User: 5.7 ms, System: 2.2 ms]
  Range (min … max):     2.5 ms …  12.9 ms    877 runs
 
Benchmark #2: target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-java-stacks-01.txt
  Time (mean ± σ):       8.5 ms ±   1.3 ms    [User: 6.1 ms, System: 2.3 ms]
  Range (min … max):     7.2 ms …  14.1 ms    238 runs
 
Summary
  'target/release/inferno-collapse-perf --all ./flamegraph/test/perf-java-stacks-01.txt' ran
    1.08 ± 0.26 times faster than 'target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-java-stacks-01.txt'


==>  ./flamegraph/test/perf-numa-stacks-01.txt  <==
Benchmark #1: target/release/inferno-collapse-perf --all ./flamegraph/test/perf-numa-stacks-01.txt
  Time (mean ± σ):       8.1 ms ±   1.8 ms    [User: 6.5 ms, System: 1.8 ms]
  Range (min … max):     3.7 ms …  14.3 ms    234 runs
 
Benchmark #2: target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-numa-stacks-01.txt
  Time (mean ± σ):       9.6 ms ±   1.3 ms    [User: 7.3 ms, System: 2.4 ms]
  Range (min … max):     4.6 ms …  15.0 ms    288 runs
 
Summary
  'target/release/inferno-collapse-perf --all ./flamegraph/test/perf-numa-stacks-01.txt' ran
    1.18 ± 0.31 times faster than 'target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-numa-stacks-01.txt'


==>  ./flamegraph/test/perf-rust-Yamakaky-dcpu.txt  <==
Benchmark #1: target/release/inferno-collapse-perf --all ./flamegraph/test/perf-rust-Yamakaky-dcpu.txt
  Time (mean ± σ):       6.3 ms ±   1.3 ms    [User: 4.7 ms, System: 1.9 ms]
  Range (min … max):     1.6 ms …  10.2 ms    353 runs
 
Benchmark #2: target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-rust-Yamakaky-dcpu.txt
  Time (mean ± σ):       6.7 ms ±   1.1 ms    [User: 4.5 ms, System: 2.4 ms]
  Range (min … max):     5.3 ms …  11.6 ms    360 runs
 
Summary
  'target/release/inferno-collapse-perf --all ./flamegraph/test/perf-rust-Yamakaky-dcpu.txt' ran
    1.05 ± 0.27 times faster than 'target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-rust-Yamakaky-dcpu.txt'


==>  ./flamegraph/test/perf-vertx-stacks-01.txt  <==
Benchmark #1: target/release/inferno-collapse-perf --all ./flamegraph/test/perf-vertx-stacks-01.txt
  Time (mean ± σ):      33.4 ms ±   6.6 ms    [User: 30.7 ms, System: 2.8 ms]
  Range (min … max):    16.8 ms …  51.0 ms    68 runs
 
Benchmark #2: target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-vertx-stacks-01.txt
  Time (mean ± σ):      30.5 ms ±   9.3 ms    [User: 27.7 ms, System: 2.9 ms]
  Range (min … max):    11.2 ms …  47.7 ms    190 runs
 
Summary
  'target/release/inferno-collapse-perf-origin --all ./flamegraph/test/perf-vertx-stacks-01.txt' ran
    1.10 ± 0.40 times faster than 'target/release/inferno-collapse-perf --all ./flamegraph/test/perf-vertx-stacks-01.txt'

@codecov-io
Copy link

codecov-io commented Feb 14, 2019

Codecov Report

Merging #46 into master will increase coverage by 0.02%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #46      +/-   ##
==========================================
+ Coverage   82.39%   82.41%   +0.02%     
==========================================
  Files           9        9              
  Lines         869      870       +1     
==========================================
+ Hits          716      717       +1     
  Misses        153      153
Impacted Files Coverage Δ
src/flamegraph/mod.rs 84.17% <100%> (+0.1%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f41a0f9...2c169b0. Read the comment docs.

@jonhoo
Copy link
Owner

jonhoo commented Feb 14, 2019

Hmm, those results don't look like this actually gives a consistent speed-up; isn't it all just lost in the noise? Sadly I think we need tafia/quick-xml#148 for this to actually give a meaningful speed-up :/

@meven
Copy link
Contributor Author

meven commented Feb 14, 2019

I agree with you, based on my performance measurement tests, the results are inconsistent and lost in noice.

@meven meven closed this Feb 14, 2019
@jonhoo
Copy link
Owner

jonhoo commented Feb 14, 2019

@meven If you want to fix #41 though, maybe you could give writing a PR for tafia/quick-xml#148 a stab though? Should be pretty easy to fix! And then you can properly fix #41 :D

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.

3 participants