search

neo4j-labs / neo4rs

Rust driver for Neo4j
https://docs.rs/neo4rs
204 stars 59 forks source link

Replace BoltMap internals with ahash/fxmap #51

Open knutwalker opened 1 year ago

thelonelyvulpes commented 1 year ago

@knutwalker if you want to flesh out the considerations I am happy to pick this up. I have a basic replacement done with aHash (https://github.com/thelonelyvulpes/neo4rs/pull/1), but I wanted to explore boltmap's traits to improve ergonomics.

knutwalker commented 1 year ago

@thelonelyvulpes thanks for the input. The other thing right now we would want to consider is https://github.com/neo4j-labs/neo4rs/issues/52, where we might want to use a Vec<(BoltString, BoltType)> instead of a map. For smaller map sizes, the linear lookup can be faster than going through hashing and finding a free slot, but we want to properly benchmark this (there is some precedent in this article). Using an ahashmap internally is a good first step regardless and we would want to benchmark against that, so I think we can go forward with this issue. We'll be happy to accept your contributions and if you want you can also split it into updating the internals end exploring the traits.

thelonelyvulpes commented 1 year ago

I have been learning a bit about Criterion should have some benchmarks to look at soon, been busy working some other pet projects so will try and tackle wrapping up this relatively small change, do we need to worry breaking changes?

knutwalker commented 1 year ago

We've a bunch of other things planned that would be breaking, at the very least an update to support 5.x and drop 4.y where y < 4, so I think we don't need to worry about it too much right now.