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

Fixes a performance bug in bytes::Regex::replace. #210

Merged
merged 1 commit into from Apr 23, 2016

Conversation

BurntSushi
Copy link
Member

This was using Vec::extend to accumulate bytes in a buffer, but this
compiles down to less efficient code than, say, Vec::extend_from_slice.
However, that method is newly available as of Rust 1.6, so we do a small
backport to regain performance.

This bug was noticed by @llogiq here: TeXitoi/benchmarksgame-rs#31
In particular, this increases the performance of bytes::Regex two-fold on
that benchmark.

This was using `Vec::extend` to accumulate bytes in a buffer, but this
compiles down to less efficient code than, say, `Vec::extend_from_slice`.
However, that method is newly available as of Rust 1.6, so we do a small
backport to regain performance.

This bug was noticed by @llogiq here: TeXitoi/benchmarksgame-rs#31
In particular, this increases the performance of bytes::Regex two-fold on
that benchmark.
@BurntSushi
Copy link
Member Author

@alexcrichton I know I'm really pushing it with this Rust 1.3 business. :-)

An alternative (which would let us omit the unsafe) would be to add a feature and enable it with a build.rs, and then use Vec::extend_from_slice when the feature is enabled and fall back to something slower if not.

@alexcrichton
Copy link
Member

Looks good to me!

@alexcrichton alexcrichton reopened this Apr 23, 2016
@alexcrichton alexcrichton merged commit ed70e16 into master Apr 23, 2016
@alexcrichton alexcrichton deleted the extend-from-slice branch April 23, 2016 00:15
@llogiq
Copy link

llogiq commented Apr 23, 2016

@BurntSushi Great!

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