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

Allow inlining standard Parse* trait impls #374

Merged
merged 1 commit into from
Apr 25, 2024

Conversation

neocturne
Copy link
Contributor

Trait impl methods need to be marked with #[inline] to allow cross-crate inlining.

As many of these functions are trivial, but they are called for every single position of the input, this has major impact:

  • Performance of the new parse-rosetta-rs JSON parser example I recently contributed ([1]) increases by almost 40% on my machine, making it slightly faster than winnow (and possibly the fastest full-featured parser listed there)
  • For a simple tokenizer I built for one of my projects, I measured a performance gain of ~94%, almost doubling the throughput!

[1] https://github.com/rosetta-rs/parse-rosetta-rs/blob/main/examples/peg-app/parser.rs

Trait impl methods need to be marked with #[inline] to allow cross-crate
inlining.

As many of these functions are trivial, but they are called for every
single position of the input, this has major impact:

- Performance of the new parse-rosetta-rs JSON parser example I recently
  contributed ([1]) increases by almost 40% on my machine, making it
  slightly faster than winnow (and possibly the fastest full-featured
  parser listed there)
- For a simple tokenizer I built for one of my projects, I measured a
  performance gain of ~94%, almost doubling the throughput!

[1] https://github.com/rosetta-rs/parse-rosetta-rs/blob/main/examples/peg-app/parser.rs
@kevinmehall kevinmehall merged commit 2fc1cad into kevinmehall:master Apr 25, 2024
2 checks passed
@kevinmehall
Copy link
Owner

Nice find, thanks!

@neocturne neocturne deleted the inline branch April 25, 2024 17:40
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.

2 participants