Skip to content
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

May we have a comment explaining why the library implements its own fixed length hashmap? #197

Open
makoConstruct opened this issue Oct 30, 2017 · 2 comments

Comments

@makoConstruct
Copy link

around about here

buckets: [Option<Box<StringCacheEntry>>; NB_BUCKETS],

I'm mostly just curious. I wonder if it would be worth considering using a parallel hashmap? In my own project, I'm eventually going to need to intern things other than strings. Is there a reason the generic interner from the syntax crate wasn't appropriate for servo's strings? Could the generic interner be improved to the point of being suitable?

@Manishearth
Copy link
Member

Is there a reason the generic interner from the syntax crate wasn't appropriate for servo's strings?

It's not something that's exposed (it's unstable).


The custom hashtable is presumably because we don't expect hashes to clash often so we're using a chained hashtable instead of a probing one. I'm not sure.

@SimonSapin
Copy link
Member

I started typing up how a custom hash map allows Atom::drop to remove a map entry a dynamic atom’s reference count hits zero, but now I think that’d also work with a standard HashMap, only the refcounting needs to be custom.

So I think HashMap would probably work. I don’t know if it’d perform better or worse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants