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

starts_with seems inefficient #138

Open
mrstux opened this issue Dec 15, 2023 · 1 comment
Open

starts_with seems inefficient #138

mrstux opened this issue Dec 15, 2023 · 1 comment

Comments

@mrstux
Copy link

mrstux commented Dec 15, 2023

I was reviewing the remove_dot_segments implementation and I noticed starts_with is implemented as:

inline bool starts_with(std::string const &str, const char *range) {
  return str.find(range) == 0;
}

Surely this is a very inefficient way to determine if a string starts with another string, as potentially it will search a very large haystack to find a needle at the very end, or not at all, when all it cares about is if the needle is at the start.

And the remove_dot_segments function calls this 5 times per segment of each uri.

perhaps the following alternative would be better, as you do know the size of each prefix upfront.

inline bool starts_with(std::string const &str, const char *prefix, size_t prefix_len) {
  return str.compare(0, prefix_len, prefix, prefix_len) == 0;
}

Of course, its better to just use a string_view, which could capture the literal string size at the callsite.

constexpr bool starts_with(std::string_view str, std::string_view prefix) noexcept {
	if (str.size() < prefix.size())
		return false;
	
	return std::string_view::traits_type::compare(str.data(), prefix.data(), prefix.size()) == 0;
}

return str.find(range) == 0;

@glynos
Copy link
Member

glynos commented Dec 16, 2023

Thanks for the review. I think the approach to use the string_view is the best of the options you provide.

I am travelling and won't have the opportunity to address this until January. I would be able to review a PR if you can prepare one.

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