-
Notifications
You must be signed in to change notification settings - Fork 563
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Previously we have had to deal with the inconvenient way in which search operations occur in the same UI thread as the rest of the app. The inconvenient part has been that while typing and when you want the app to be most responsive - that is when it was the slowest due to the filtering happening in the note list. For many years this was such a problem and would make typing in the search field too cumbersome so we created a debounce value to wait until you stopped typing in order to update the note list. This debounce, carried out as a separate data stream from the main app state, caused problems in that the debounced search value and the app state could get out of sync with each other. This led to even more confusion creating separate values for the text in the search field _and_ a debounced search field value. With recent refactorings to the app state we have been able to eliminate some of the spurious data flows and get back to a single state value representing the currently-typed value in the search field. We have separated the act of updating the note list from the act of typing in a search query. With that final necessary separate we are able to get to this patch. In this patch we're recreating the search system and moving it into a WebWorker, outside of the main UI thread. What this means is that we can now update the filtering of the note list as often as we want and it won't interfere with the search field or with typing. By moving the data into its own asynchronous "search indexer" too we are able to make several trivial optimizations which have resulted in improving the performance of searching by several magnitudes. These mostly revolve around doing less work by being reactive instead of repeating calculations on every keypress: - Instead of building a case-insensitive search RegExp, store the lower-cased version of each note in the search index and lower-case the search field and use more basic string search functions. - Instead of parsing the search query every time we execute a search/filter operation, only parse it when it changes. - Instead of scanning the list of note tags on every search, build a `Set()` when we index the note which provides O(1) membership tests instead of O(N) with the number of tags in a note. - Provide a guard against long-running searches by performing a "quick" search up until 10ms have passed and then queuing a longer full search if we hit that threshold. (In all my testing I could not find a search that took more than about 1ms). In hind sight a few of the optimizations in this PR could have been merged into `develop` directly without the WebWorker and because of the performance impact we could probably get by without making search asynchronous. However, there is still a clear benefit to pushing this into a WebWorker and out of the main thread. As we continue to iterate on the quality of the search results this will become more important. Props to @belcherj and @codebykat for their hard work refactoring the app state which has enabled this kind of work to be done.
- Loading branch information
Showing
12 changed files
with
294 additions
and
64 deletions.
There are no files selected for viewing
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,109 @@ | ||
import SearchWorker from 'worker-loader!./worker'; | ||
|
||
import actions from '../state/actions'; | ||
|
||
import * as A from '../state/action-types'; | ||
import * as S from '../state'; | ||
import * as T from '../types'; | ||
|
||
const emptyList = [] as T.NoteEntity[]; | ||
|
||
export const middleware: S.Middleware = store => { | ||
const searchWorker = new SearchWorker(); | ||
const { | ||
port1: searchProcessor, | ||
port2: _searchProcessor, | ||
} = new MessageChannel(); | ||
|
||
searchProcessor.onmessage = event => { | ||
switch (event.data.action) { | ||
case 'filterNotes': | ||
store.dispatch( | ||
actions.ui.filterNotes( | ||
store | ||
.getState() | ||
.appState.notes?.filter(({ id }) => event.data.noteIds.has(id)) || | ||
emptyList | ||
) | ||
); | ||
break; | ||
} | ||
}; | ||
|
||
searchWorker.postMessage('boot', [_searchProcessor]); | ||
let hasInitialized = false; | ||
|
||
return next => (action: A.ActionType) => { | ||
const result = next(action); | ||
|
||
switch (action.type) { | ||
case 'App.notesLoaded': | ||
if (!hasInitialized) { | ||
const { | ||
appState: { notes }, | ||
} = store.getState(); | ||
if (notes) { | ||
notes.forEach(note => | ||
searchProcessor.postMessage({ | ||
action: 'updateNote', | ||
noteId: note.id, | ||
data: note.data, | ||
}) | ||
); | ||
} | ||
|
||
hasInitialized = true; | ||
} | ||
searchProcessor.postMessage({ action: 'filterNotes' }); | ||
break; | ||
|
||
case 'REMOTE_NOTE_UPDATE': | ||
searchProcessor.postMessage({ | ||
action: 'updateNote', | ||
noteId: action.noteId, | ||
data: action.data, | ||
}); | ||
break; | ||
|
||
case 'App.selectTag': | ||
searchProcessor.postMessage({ | ||
action: 'filterNotes', | ||
openedTag: action.tag.data.name, | ||
}); | ||
break; | ||
|
||
case 'App.selectTrash': | ||
searchProcessor.postMessage({ | ||
action: 'filterNotes', | ||
openedTag: null, | ||
showTrash: true, | ||
}); | ||
break; | ||
|
||
case 'App.showAllNotes': | ||
searchProcessor.postMessage({ | ||
action: 'filterNotes', | ||
openedTag: null, | ||
showTrash: false, | ||
}); | ||
break; | ||
|
||
case 'SEARCH': | ||
searchProcessor.postMessage({ | ||
action: 'filterNotes', | ||
searchQuery: action.searchQuery, | ||
}); | ||
break; | ||
|
||
case 'DELETE_NOTE_FOREVER': | ||
case 'RESTORE_NOTE': | ||
case 'TRASH_NOTE': | ||
case 'App.authChanged': | ||
case 'App.trashNote': | ||
searchProcessor.postMessage({ action: 'filterNotes' }); | ||
break; | ||
} | ||
|
||
return result; | ||
}; | ||
}; |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,127 @@ | ||
import { getTerms } from '../../utils/filter-notes'; | ||
|
||
import * as T from '../../types'; | ||
|
||
const notes: Map< | ||
T.EntityId, | ||
[T.EntityId, T.Note & { tags: Set<T.TagName> }] | ||
> = new Map(); | ||
|
||
self.onmessage = bootEvent => { | ||
const mainApp: MessagePort | undefined = bootEvent.ports[0]; | ||
|
||
if (!mainApp) { | ||
// bail if we don't get a custom port | ||
return; | ||
} | ||
|
||
let searchQuery = ''; | ||
let searchTerms: string[] = []; | ||
let filterTags = new Set<T.TagName>(); | ||
let showTrash = false; | ||
|
||
const tagsFromSearch = (query: string) => { | ||
const tagPattern = /(?:\btag:)([^\s,]+)/g; | ||
const searchTags = new Set<T.TagName>(); | ||
let match; | ||
while ((match = tagPattern.exec(query)) !== null) { | ||
searchTags.add(match[1].toLocaleLowerCase()); | ||
} | ||
return searchTags; | ||
}; | ||
|
||
const updateFilter = (scope = 'quickSearch') => { | ||
const tic = performance.now(); | ||
const matches = new Set<T.EntityId>(); | ||
|
||
for (const [noteId, note] of notes.values()) { | ||
// return a small set of the results quickly and then | ||
// queue up another search. this improves the responsiveness | ||
// of the search and it gives us another opportunity to | ||
// receive inbound messages from the main thread | ||
// in testing this was rare and may only happen in unexpected | ||
// circumstances such as when performing a garbage-collection | ||
const toc = performance.now(); | ||
if (scope === 'quickSearch' && toc - tic > 10) { | ||
mainApp.postMessage({ action: 'filterNotes', noteIds: matches }); | ||
queueUpdateFilter(0, 'fullSearch'); | ||
return; | ||
} | ||
|
||
if (showTrash !== note.deleted) { | ||
continue; | ||
} | ||
|
||
let hasAllTags = true; | ||
for (const tagName of filterTags.values()) { | ||
if (!note.tags.has(tagName)) { | ||
hasAllTags = false; | ||
break; | ||
} | ||
} | ||
if (!hasAllTags) { | ||
continue; | ||
} | ||
|
||
if ( | ||
searchTerms.length > 0 && | ||
!searchTerms.every(term => note.content.includes(term)) | ||
) { | ||
continue; | ||
} | ||
|
||
matches.add(noteId); | ||
} | ||
|
||
mainApp.postMessage({ action: 'filterNotes', noteIds: matches }); | ||
}; | ||
|
||
let updateHandle: ReturnType<typeof setTimeout> | null = null; | ||
const queueUpdateFilter = (delay = 0, searchScope = 'quickSearch') => { | ||
if (updateHandle) { | ||
clearTimeout(updateHandle); | ||
} | ||
|
||
updateHandle = setTimeout(() => { | ||
updateHandle = null; | ||
updateFilter(searchScope); | ||
}, delay); | ||
}; | ||
|
||
mainApp.onmessage = event => { | ||
if (event.data.action === 'updateNote') { | ||
const { noteId, data } = event.data; | ||
|
||
const noteTags = new Set(data.tags.map(tag => tag.toLocaleLowerCase())); | ||
notes.set(noteId, [ | ||
noteId, | ||
{ | ||
...data, | ||
content: data.content.toLocaleLowerCase(), | ||
tags: noteTags, | ||
}, | ||
]); | ||
|
||
queueUpdateFilter(1000); | ||
} else if (event.data.action === 'filterNotes') { | ||
if ('string' === typeof event.data.searchQuery) { | ||
searchQuery = event.data.searchQuery.trim().toLocaleLowerCase(); | ||
searchTerms = getTerms(searchQuery); | ||
filterTags = tagsFromSearch(searchQuery); | ||
} | ||
|
||
if ('string' === typeof event.data.openedTag) { | ||
filterTags = tagsFromSearch(searchQuery); | ||
filterTags.add(event.data.openedTag.toLocaleLowerCase()); | ||
} else if (null === event.data.openedTag) { | ||
filterTags = tagsFromSearch(searchQuery); | ||
} | ||
|
||
if ('boolean' === typeof event.data.showTrash) { | ||
showTrash = event.data.showTrash; | ||
} | ||
|
||
queueUpdateFilter(); | ||
} | ||
}; | ||
}; |
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,7 @@ | ||
{ | ||
"extends": "../../../tsconfig.json", | ||
"compilerOptions": { | ||
"isolatedModules": false, | ||
"lib": ["esnext", "webworker"] | ||
} | ||
} |
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
import * as auth from './auth/actions'; | ||
import * as settings from './settings/actions'; | ||
import * as simperium from './simperium/actions'; | ||
import * as ui from './ui/actions'; | ||
|
||
export default { | ||
auth, | ||
simperium, | ||
settings, | ||
ui, | ||
}; |
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
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import * as A from '../action-types'; | ||
import * as T from '../../types'; | ||
|
||
export const remoteNoteUpdate: A.ActionCreator<A.RemoteNoteUpdate> = ( | ||
noteId: T.EntityId, | ||
data: T.Note | ||
) => ({ | ||
type: 'REMOTE_NOTE_UPDATE', | ||
noteId, | ||
data, | ||
}); |
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.