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

Store needle length #52

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Store needle length #52

wants to merge 2 commits into from

Conversation

snoyes
Copy link

@snoyes snoyes commented Nov 26, 2019

Profiling a full screen search shows that calling len(needle) repeatedly adds nearly a second to the run time.

Profiling a full screen search shows that calling len repeatedly adds nearly a second to the run time.
Copy link

@TrangOul TrangOul left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The same should be done in _steppingFind - len(needle) is called repeatedly in a loop.

@snoyes
Copy link
Author

snoyes commented Feb 16, 2021

The same should be done in _steppingFind - len(needle) is called repeatedly in a loop.

Do you think that's why no performance gain was noted and so the step size was hardcoded to 1, hence _steppingFind is never used?

# NOTE: After running tests/benchmarks.py on the following code, it seem that having a step
# value greater than 1 does not give *any* significant performance improvements.
# Since using a step higher than 1 makes for less accurate matches, it will be
# set to 1.
step = 1 # hard-code step as 1 until a way to improve it can be figured out.
if step == 1:
firstFindFunc = _kmp
else:
firstFindFunc = _steppingFind

@Avasam
Copy link

Avasam commented Aug 21, 2024

@snoyes This now has conflicts

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.

3 participants