search

helix-editor / nucleo

A fast and convenient fuzzy matcher library for rust
Mozilla Public License 2.0
899 stars 28 forks source link

reset matches #10

Closed poliorcetics closed 1 year ago

poliorcetics commented 1 year ago
pascalkuthe commented 1 year ago

feat: extend is not specialized here, workaround this by reserving in advance

That's unfortunate but a good catch! Rust iterator make this a bit too easy to miss for my liking. Oh well

pascalkuthe commented 1 year ago

So I have been looking inti this a bit more. Vec::extend does always use the size_hint for reserving space in the vector. The specialization accelerates some iterators further but not by a huge amount and the reserve wouldnt help with that.

Your old implementation actually caused the matches vector to just inifinitly grow in most cases. My guess is that really the resve may help somewhere else but it could also just be a wrong measurement since this kind of concurrent code (with a timeout) is pretty hard to benchmark. So I am leaning towards not merging this.

You mentioned that you could reproduce a perf improvement. Could you share the testcase and his you measured the performance diff. I aught to setup a benchmark for the high klvel matcher anyway so this might be a good chance to do that here.

I think this branch can likely be optimized further (using par_extend and doing the scoring right in an par_extend so avoiding the double iteration)

poliorcetics commented 1 year ago

I always got .6ms yesterday, I don't this morning, I removed the commit