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.

Sign up for GitHub

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

buffer: make indexOf faster #1059

Closed
wants to merge 1 commit into from
+31 −6
Closed

buffer: make indexOf faster #1059

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 30 additions & 6 deletions src/node_buffer.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -607,13 +607,37 @@ int32_t IndexOf(const char* haystack,
const char* needle,
size_t n_length) {
CHECK_GE(h_length, n_length);
// TODO(trevnorris): Implement Boyer-Moore string search algorithm.
for (size_t i = 0; i < h_length - n_length + 1; i++) {
if (haystack[i] == needle[0]) {
if (memcmp(haystack + i, needle, n_length) == 0)
return i;
}

const char* inc;
const void* ptr;

// Guard against zero length searches
if (h_length == 0 || n_length == 0 || n_length > h_length)
return -1;

// Single character case
if (n_length == 1) {
ptr = memchr(haystack, *needle, h_length);
if (ptr == nullptr)
return -1;
return static_cast<int32_t>(static_cast<const char*>(ptr) - haystack);
}

// Do multicharacter string match
inc = haystack;
do {
ptr = memchr(inc, *needle, h_length - static_cast<int32_t>(inc - haystack));
if (ptr != nullptr) {
const void* ptr2 = memmem(inc,
Copy link
Member

Choose a reason for hiding this comment

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

Is it available on windows?

Copy link
Member

Choose a reason for hiding this comment

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

Guess it isn't: https://msdn.microsoft.com/en-us/library/aa246468(v=vs.60).aspx

Copy link
Contributor Author

Choose a reason for hiding this comment

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

suck, seriously? alright, back to the drawing board.

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just pick a "good" algorithm from here to use on Windows (or everywhere for consistency?). Start with Turbo Boyer-Moore or Boyer-Moore-Horspool perhaps?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@mscdex TBH this implementation was thrown together in 10 mins, and is by no means a good implementation. i'll take the time to read up more on proper Boyer-Moore implementations. Thanks for the links.

h_length - static_cast<int32_t>(inc - haystack),
needle,
n_length);
if (ptr2 != nullptr)
return static_cast<int32_t>(static_cast<const char*>(ptr2) - haystack);
inc = static_cast<const char*>(ptr);
}
} while (ptr != nullptr);

return -1;
}

Expand Down
1 change: 1 addition & 0 deletions test/parallel/test-buffer-indexof.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ assert.equal(b.indexOf('bc', -5), 1);
assert.equal(b.indexOf('bc', NaN), 1);
assert.equal(b.indexOf('bc', -Infinity), 1);
assert.equal(b.indexOf('bc', Infinity), -1);
assert.equal(b.indexOf('cd'), 2);
assert.equal(b.indexOf('f'), b.length - 1);
assert.equal(b.indexOf('z'), -1);
assert.equal(b.indexOf(''), -1);
Expand Down