search

rocicorp / repc

The canonical Replicache client, implemented in Rust.
Other
30 stars 7 forks source link

Use binary search for has and get #216

Closed arv closed 4 years ago

arv commented 4 years ago

First look in the pending, then binary search the flatbuffers::Vector.

The std binary_search does not work on flatbuffers::Vector (at least I could not figure out how to do it without a copy) so I copied and modified the code.

arv commented 4 years ago

Got side tracked... but I had some fun!

aboodman commented 4 years ago

Nice but I kind of go back and forth on this. Once we have prolly trees then we should only have a small number of items in each node. So due to cpu caching I would be surprised if this makes it any faster or could even make it worse. It also increases complexity significantly

I kinda feel like in Rust we want to conserve our complexity budget wherever we can 😬. @phritz ?

phritz commented 4 years ago

Generally in favor of less complexity in rust. In this case seems like it should improve performance for now and the complexity is at least concentrated and doesn't have to be understood to use the thingy, so in favor of merging this.

aboodman commented 4 years ago

OK, LGTM then

On Fri, Oct 16, 2020 at 10:08 AM Phritz notifications@github.com wrote:

Generally in favor of less complexity in rust. In this case seems like it should improve performance for now and the complexity is at least concentrated and doesn't have to be understood to use the thingy, so in favor of merging this.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/rocicorp/repc/pull/216#issuecomment-710536972, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAATUBCBH75SGQCWY4QXAD3SLCRZ7ANCNFSM4SRQLG7Q .