Micro-optimize search #482
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
With newer versions of npm, the node_modules folder is flattened. Instead of reaching into grunt-mocha/node_modules to find mocha.css and mocha.js, we need to look directly in the mocha folder.
By doing this, we make `setSearchString` twice as fast. It's already pretty fast, but this won't hurt.
By memoizing a RegExp instead of matching against a string, the total time to search goes down with about 30-60%. This is the total search times for strings "l" and "le" on a list of 1000 items (most items named "Leia <n>", where <n> is a incremented number). Without RegExp memoization: first run: 3.2ms second run: 1.63ms With RegExp memoization: first run: 2.16ms second run: 0.7ms
Instead of converting to lower-case before we match a string, we can tell the regexp to do this for us. This makes searching a little bit faster, roughly 25% in my local testing.
Since we don't care about where in the string the match was, we can use RegExp.test directly (which returns a boolean). This might make searching a little faster, although I didn't see any significant signs of that in my local testing.
trotzig
commented
Jan 13, 2017
| search.reset(); | ||
| } else { | ||
| list.searched = true; | ||
| if (customSearch) { | ||
| customSearch(searchString, columns); | ||
| customSearch(searchPattern, columns); |
There was a problem hiding this comment.
This might be a breaking change, depending on how people use the customSearch option.
There was a problem hiding this comment.
searchPattern.source could help.
By using a range, we can improve performance of templater.clear() by roughly 25% (tested in Chrome). I decided to make this perf enhancement opt-in to browsers supporting the `document.createRange` method. It's not supported in IE<9, but most modern browsers should have it. On a list with 1000 items, this change took down the time it takes for templater.clear() from roughly 40ms to <30ms. In total, the whole search execution takes about 160ms, so this shaves off ~6% of execution time.
By adding DOM nodes one by one, we're causing multiple reflows in the browser. This can have bad effects on performane when lists are longs. By adding DOM nodes to a fragment we can limit the number of reflows to one. This cuts execution time for list.update() in half (40ms to 20ms) on a list with 1000 items. While I was in the code, I noticed a misspelled comment that I fixed.
YZirkind
reviewed
Oct 15, 2020
src/search.js
Outdated
| prepare.setOptions(arguments); // str, cols|searchFunction, searchFunction | ||
| prepare.setColumns(); | ||
|
|
||
| if (searchString === "" ) { | ||
| if (!searchPattern ) { |
There was a problem hiding this comment.
searchPattern will never be null:
console.log(new RegExp());
// output: /(?:)/
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR started out with me exploring whether to move some of the functions used when searching over to a web worker, but ended up just making a few minor improvements to the matching behavior. I call it "micro-optimize" because the DOM manipulation is still the most expensive part of list.js searching.