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

Add inlining to pnet_macros_support packets.rs #553

Closed
infosechoudini opened this issue Apr 19, 2022 · 3 comments
Closed

Add inlining to pnet_macros_support packets.rs #553

infosechoudini opened this issue Apr 19, 2022 · 3 comments

Comments

@infosechoudini
Copy link

We should add inlining to all functions that are not part of traits in pnet_macros_support/src/packets.rs

While running benchmarks, it seems to increase the overall speed by 100% for functions that require getting or setting parts of the packet struct.

Without inlining
running 5 tests
test bench_packet_get_source             ... bench:          15 ns/iter (+/- 2)
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:          17 ns/iter (+/- 0)
With inlining
running 5 tests
test bench_packet_get_source             ... bench:           8 ns/iter (+/- 1)
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 (+/- 3)

by far the speed to parse packets is much slower than nom and nom_derive. This will increase usability of the crate.

@infosechoudini
Copy link
Author

additionally add to pnet_macros/src/decorators to be able to handle packet generation better. The above bench with inlining includes that

@infosechoudini
Copy link
Author

infosechoudini commented Apr 19, 2022

#555 did this PR for this

@infosechoudini
Copy link
Author

Closing! PR Merged!

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

No branches or pull requests

1 participant