-
Notifications
You must be signed in to change notification settings - Fork 548
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
Allowing to load data without serializing to JSON first. #115
Comments
Updated lunr.Store to call toJSON on lunr.SortedSet.
Why is this better than just storing the stringified JSON output in IndexedDB? Just trying to understand the use case where this is a better solution. |
Speed. Calling |
Would you mind providing some benchmarks showing the difference in performance between this implementation and the current implementation? Are you sure that the slowness comes from |
I went ahead and did a crude benchmark and I don't really know what to make of the results. In some aspects your code and using JSON.stringify is faster, but in some aspects it is not. For reference, here is the fiddle: http://jsfiddle.net/0qvo6kuo/16/ The numbers will be stored in the console. If the word "json" appears, then the data was stringified/parsed. If the word "save" appears, then the data was saved/retrieved from the IndexedDB. My issue with the numbers is that, even though my implementation is longer, it doesn't block the UI thread. What if we introduced a new method that allowed for saving/restoring Lunr without JSON? That way we could keep the toJSON implementation, which is fast for JSON, and have another implementation that will just dump out the data. Let me know your thoughts. |
Hello.
I'm storing data using IndexedDB, which does not need to be serialized to JSON first. However, Lunr relies heavily on the automatic call of
toJSON
byJSON.stringify
. Because of this, callinglunr.Index.load(index.toJSON)
does not work as expected. This exact issue has come up previously (#52).After review of the code, the only instance that needed to be updated was
toJSON
onlunr.Store
.This is my proposed code update:
I'll be making a pull request shortly.
The text was updated successfully, but these errors were encountered: