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

Changes to Functions to include inline and moved benches to criterion to use stable toolchain #555

Merged
merged 2 commits into from Apr 25, 2022

Conversation

infosechoudini
Copy link

@infosechoudini infosechoudini commented Apr 19, 2022

pent_macro_support/src/packet.rs

  • added inlining to all functions available to increase speed at cost of size

pnet_macro/src/decorator.rs

  • added inlining to all functions to increase speed at cost of size

pnet_packet/

  • moved benches from ethernet.rs to new folder ~/benches/packet_benchmarks.rs

Benchmarks Before Changes

test bench_ipv4_parsing                  ... bench:           8 ns/iter (+/- 0)
test bench_packet_get_source             ... bench:          16 ns/iter (+/- 0)
test bench_packet_immutable_to_immutable ... bench:          10 ns/iter (+/- 1)
test bench_packet_mutable_to_immutable   ... bench:          10 ns/iter (+/- 0)
test bench_packet_new_constructor        ... bench:           8 ns/iter (+/- 0)
test bench_packet_set_source_black_box   ... bench:          18 ns/iter (+/- 1)
Benchmarks After Changes
running 6 tests
test bench_ipv4_parsing                  ... bench:           1 ns/iter (+/- 0)
test bench_packet_get_source             ... bench:           7 ns/iter (+/- 0)
test bench_packet_immutable_to_immutable ... bench:           2 ns/iter (+/- 0)
test bench_packet_mutable_to_immutable   ... bench:           2 ns/iter (+/- 0)
test bench_packet_new_constructor        ... bench:           1 ns/iter (+/- 0)
test bench_packet_set_source_black_box   ... bench:           9 ns/iter (+/- 0)

Harry Thomas added 2 commits Apr 19, 2022
…benches/packet_benchmarks.rs; added inlining to pnet_macros to increase speed of parsing data into packet
@infosechoudini
Copy link
Author

@infosechoudini infosechoudini commented Apr 19, 2022

Criterion Output

Gnuplot not found, using plotters backend
EthernetPacket New Packet                                                                             
                        time:   [1.9560 ns 1.9666 ns 1.9787 ns]
                        change: [-0.5771% +0.3666% +1.2536%] (p = 0.43 > 0.05)
                        No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
  5 (5.00%) high mild
  2 (2.00%) high severe

EthernetPacket Get Source                                                                             
                        time:   [4.5342 ns 4.5548 ns 4.5782 ns]
                        change: [+0.1741% +1.5730% +2.5907%] (p = 0.00 < 0.05)
                        Change within noise threshold.
Found 13 outliers among 100 measurements (13.00%)
  8 (8.00%) high mild
  5 (5.00%) high severe

EthernetPacket Set Source                                                                             
                        time:   [637.33 ps 638.82 ps 640.36 ps]
                        change: [-3.0706% -2.4471% -1.8802%] (p = 0.00 < 0.05)
                        Performance has improved.
Found 4 outliers among 100 measurements (4.00%)
  4 (4.00%) high mild

Mutable to Immutable    time:   [2.5357 ns 2.5486 ns 2.5620 ns]                                  
                        change: [-1.5676% -0.7663% -0.0239%] (p = 0.06 > 0.05)
                        No change in performance detected.
Found 7 outliers among 100 measurements (7.00%)
  6 (6.00%) high mild
  1 (1.00%) high severe

Immutable to Immutable  time:   [2.5807 ns 2.5930 ns 2.6076 ns]                                    
                        change: [-1.4937% -0.6850% +0.0496%] (p = 0.09 > 0.05)
                        No change in performance detected.
Found 9 outliers among 100 measurements (9.00%)
  5 (5.00%) high mild
  4 (4.00%) high severe

IPV4 Parsing            time:   [1.9412 ns 1.9472 ns 1.9535 ns]                          
                        change: [+0.0893% +0.6524% +1.2177%] (p = 0.03 < 0.05)
                        Change within noise threshold.
Found 8 outliers among 100 measurements (8.00%)
  4 (4.00%) high mild
  4 (4.00%) high severe

@infosechoudini infosechoudini changed the title Benches Changes to Functions to include inline and moved benches to criterion to use stable toolchain Apr 19, 2022
@infosechoudini
Copy link
Author

@infosechoudini infosechoudini commented Apr 21, 2022

@mrmonday et al., can i get a workflow approval?

@stappersg
Copy link
Contributor

@stappersg stappersg commented Apr 21, 2022

@mrmonday et al., can i get a workflow approval?

The request is be seen by a human.
My advice is to ask again in about week.
Consider meanwhile the idea of something like easter vacaction or any other possible reason for a delay in response.

My real message is

@infosechoudini aim for the long run, avoid to get worn down on the short run

Other thing

Day changed to 20 apr 2022
00:22 -!- infosechoudini [~infosecho@h597.436.156.298.static.ip.redacted.net] has joined #libpnet
00:24 < infosechoudini> hey, i'm looking to help contribute to libpnet, was wondering what's the process
02:21 -!- infosechoudini [~infosecho@h597.436.156.298.static.ip.redacted.net] has quit [Quit: Connection closed]

Feel welcome to hang-out at libera/#libpnet IRC channel.

@infosechoudini
Copy link
Author

@infosechoudini infosechoudini commented Apr 21, 2022

@stappersg thanks, i apologize for the ping. I forgot it was easter week!!!

@mrmonday
Copy link
Contributor

@mrmonday mrmonday commented Apr 25, 2022

I'm surprised the annotations make so much difference - in the past we've removed lots of them because the compiler is "sufficiently smart" that it made no difference.

It would be good to get the benchmarks in CI, no need to do that here though.

Thanks!

@mrmonday mrmonday merged commit a8cb175 into libpnet:master Apr 25, 2022
3 checks passed
@infosechoudini infosechoudini deleted the benches branch Apr 25, 2022
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

3 participants