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

[5.3] Optimize Str::random() by just using strlen() and substr() #15104

Closed
wants to merge 1 commit into from
Closed

[5.3] Optimize Str::random() by just using strlen() and substr() #15104

wants to merge 1 commit into from

Conversation

vlakoff
Copy link
Contributor

@vlakoff vlakoff commented Aug 28, 2016

Figures, for only 10,000 iterations:

  • before: 963 ms
  • after: 887 ms

So, Str::random() is quite slow, this PR brings a speedup of about 8% which I think is worth it.

The substr() being applied on a base64 output, the change is safe and straightforward.

@vlakoff vlakoff changed the title [5.3] Optimize Str::random() by just using substr() [5.3] Optimize Str::random() by just using strlen() and substr() Aug 28, 2016
@vlakoff
Copy link
Contributor Author

vlakoff commented Aug 28, 2016

Updated the PR by making the same change to the strlen().

New figures:

  • before: 965 ms
  • after: 786 ms

18% faster. 🚀

@vlakoff
Copy link
Contributor Author

vlakoff commented Aug 28, 2016

Applied the same change to Str::quickRandom().

Also, refs #12957 and #14780.

@GrahamCampbell
Copy link
Member

This is a bug that was introduced by the bad PR that you referenced. Please could you send this to 5.1. :)

@GrahamCampbell
Copy link
Member

Could you also please update the quickRandom function not use the slow mb functions?

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