-
-
Notifications
You must be signed in to change notification settings - Fork 28
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
Use of Set for dictionaries #16
Comments
I did some quick performance testing and it seems that |
I also did some quick perf (Node v8.16.0) testing and it found that :
That's strange, in your test |
I think it's because those benchmarks (which I copied from someone else) also have a value check ( The problem with something like It looks like they are pretty similar so let's just go all-in on |
For the prefix checks, I will write a little FST memory structure which will make those much faster, in the meantime they can just use iterators and it will be slow. Performance overall is pretty good, although some complex queries are getting near 10ms which is not great. |
Added FST in #17 |
I tried your branch, it use much more memory (415MB) and seems to be slower than Set (4 times slower than Set) 🤔 Here is the test : https://gist.github.com/Joxit/32ccd5b5f63b474f30e707d804cbda25 Maybe in the future if we will want to add some metadata e.g if the token is |
Initially, I used a js object and
hasOwnProperty
to do the hashmap lookups and then later used Set andhas()
.It would be nice to standardize this, I'm just not familiar with the performance of
Set
vs.Object
, I think if Set is faster/the same then we should use it.I think one benefit of
Set
is thatObject
can possibly have issues with numeric keys?cc/ @Joxit thoughts?
The text was updated successfully, but these errors were encountered: