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

reimplement find_bm() using std::search #33

Merged
merged 1 commit into from
Jan 19, 2024

Conversation

lukem
Copy link
Contributor

@lukem lukem commented Dec 29, 2023

Use std::search() to implement find_bm() instead of using a local implementation of Boyer-Moore.

Avoids integer overflow reported in issue #32 and PR #31. Should fix build problem in issue #7.

std::search is also faster for the test program in issue #32 on a system with an Intel Xeon E-2224 CPU:

  • gcc 8.5, find_bm(): 3.16s
  • g++ 8.5, std::search: 2.40s
  • g++ 13, std::search: 2.16s

Experiments using the C++17 std::boyer_moore_searcher or std::boyer_moore_horspool_searcher were also slower than std::search in this experiment.

Use std::search() to implement find_bm() instead of using a
local implementation of Boyer-Moore.

Avoids integer overflow reported in issue tat#31 and PR tat#31.
Should fix build problem in issue tat#7.

std::search is also faster for the test program in issue tat#31
on a system with an Intel Xeon E-2224 CPU:
- gcc 8.5, find_bm(): 3.16s
- g++ 8.5, std::search: 2.40s
- g++ 13, std::search: 2.16s

Experiments using the C++17 std::boyer_moore_searcher or
std::boyer_moore_horspool_searcher were also slower than
std::search in this experiment.
@lukem
Copy link
Contributor Author

lukem commented Dec 29, 2023

I've also previously submitted PR #30 that fixes various issues detected with newer g++ versions with -Wall -Werror.

@lukem
Copy link
Contributor Author

lukem commented Dec 29, 2023

Before reimplementing with std::search, I experimented with other solutions including using the iterator's size_type instead of int, but they ended up being more complex/fragile.
std::search is also faster!

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