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

Rewrite str::push_char in rust. #2062

Closed
wants to merge 1 commit into from
Closed

Rewrite str::push_char in rust. #2062

wants to merge 1 commit into from

Conversation

grahame
Copy link
Contributor

@grahame grahame commented Mar 28, 2012

Avoid crossing to C to reallocate underlying vec when possible,
if we must we now only cross once per char (not once per byte.)
As str::from_chars calls this method, but first reserves space in the
underlying vec, str::from_chars becomes significantly quicker.

From my testing this is a decent performance win.

Avoid crossing to C to reallocate underlying array when possible,
if we must we now only cross once per char (not once per byte.)
@grahame
Copy link
Contributor Author

grahame commented Mar 28, 2012

(Sorry about all the mut_offset calls; I tried using a bind but it was slow.

@brson
Copy link
Contributor

brson commented Mar 29, 2012

Well, I've gotten sufficiently motivated to improve our various reserve API's so I will take this as is and make the requested adjustments myself. Thanks! This looks great.

@grahame
Copy link
Contributor Author

grahame commented Mar 29, 2012

Great, thanks!

I got the idea to use the upcall_vec_grow from marijn on IRC, this is my first exposure to the innards of the runtime. I'll watch your changes with interest to see how I should have done it :-)

@brson
Copy link
Contributor

brson commented Mar 29, 2012

Integrated. Thanks!

0d5d2e5 is the tweak that I did. It just reuses some existing functions (and some new ones) instead of casting to a vec and poking at the fill and using the upcall. I believe it maintains the same performance characteristics. If you see otherwise let me know.

@brson brson closed this Mar 29, 2012
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