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

isabs performance improvement #25

Closed
swahtz opened this issue Apr 25, 2019 · 1 comment
Closed

isabs performance improvement #25

swahtz opened this issue Apr 25, 2019 · 1 comment
Assignees

Comments

@swahtz
Copy link

swahtz commented Apr 25, 2019

Functions like pystring::os::path::isabs_posix call pystring::startswith:

bool isabs_posix(const std::string & s) { return pystring::startswith(s, "/"); }

And it seems that this will create a new std::string for "/" each time this is called which will dominate the execution time of code that calls isabs() repeatedly. Would it be reasonable to declare some of these constants to amortize this cost?

bool isabs_posix(const std::string & s) { static const std::string slash("/"); return pystring::startswith(s, slash); }

It seems "/" and "\" is used several times in pystring as the arguments to startswith, endswith, strip, etc. which take references to std::string as arguments and this could be more optimal for the cases where these functions get called a lot?

@grdanny
Copy link
Collaborator

grdanny commented Nov 11, 2019

Sorry for the late reply on this.

I did some measurements, and on 2 million isab_posix calls this is the difference:

Without const: 473 miliseconds
With const: 260 miliseconds

So while it is almost twice as fast with the const, it's still pretty fast without it.
Still - if you look at all the cases where we use "/", "." etc, it could improve performance on large datasets if changed it all to consts.

Let me go over all the code and see how many of these we can change to consts.

Thanks for the suggestion.

@grdanny grdanny self-assigned this Feb 2, 2020
grdanny added a commit that referenced this issue Feb 4, 2020
#25 declaring global string constants for performance improvement
@grdanny grdanny closed this as completed Feb 10, 2020
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

2 participants