perf: simplify the search algorithm #178
Conversation
|
Shouldn't we take the opportunity to add tests if we start to modify the implementation? |
|
Coverage of this file is already at 100% but I could add more once it's finished I won't change behavior |
My bad i was assuming tests were siblings of the vendored lib. |
| @@ -168,15 +169,15 @@ SBMH.prototype._sbmh_feed = function (data) { | |||
|
|
|||
| // Lookbehind buffer is now empty. We only need to check if the | |||
| // needle is in the haystack. | |||
| pos = data.indexOf(needle, pos + ((pos >= 0) * this._bufpos)) | |||
There was a problem hiding this comment.
(pos >= 0) is pointless as pos is never negative here (lookbehind is emptied)
| return (this._bufpos = pos + needleLength) | ||
| } | ||
|
|
||
| pos = len - needleLength |
There was a problem hiding this comment.
When needle is larger than the incoming data buffer, pos becomes negative here and there are many redundant/duplicate comparisons
At this part pos should only look at the incoming data (lookbehind is emptied), so it cannot be a negative integer
| ( | ||
| data[pos] !== needle[0] || | ||
| Buffer.compare( | ||
| data.subarray(pos, pos + len - pos), |
There was a problem hiding this comment.
pos + len - pos === len
| ( | ||
| data[pos] !== needle[0] || | ||
| Buffer.compare( | ||
| data.subarray(pos, pos + len - pos), | ||
| needle.subarray(0, len - pos) |
There was a problem hiding this comment.
We can start with 1 as the first character is already checked on the check just above
| this._lookbehind_size = len - pos | ||
| } | ||
|
|
||
| // Everything until pos is guaranteed not to contain needle data. | ||
| if (pos > 0) { this.emit('info', false, data, this._bufpos, pos < len ? pos : len) } |
There was a problem hiding this comment.
pos < len ? pos : len is pointless as pos can never surpass len here
There was a problem hiding this comment.
Copilot reviewed 1 out of 1 changed files in this pull request and generated no suggestions.
deps/streamsearch/sbmh.js
Outdated
| return (this._bufpos = pos + needleLength) | ||
| } | ||
|
|
||
| pos = len - needleLength | ||
| pos = Math.max(len - needleLastCharIndex, 0) |
There was a problem hiding this comment.
| pos = Math.max(len - needleLastCharIndex, 0) | |
| pos = len - needleLastCharIndex | |
| pos = pos < 0 ? 0 |
Math.max is unfortunately slower than if statement
Removes unnecessary checks