georust / rstar

R*-tree spatial index for the Rust ecosystem
https://docs.rs/rstar
Apache License 2.0
384 stars 67 forks source link

Use unstable sort for envelopes and node reinsertion #160

Closed grovesNL closed 4 months ago

grovesNL commented 4 months ago

I was just looking at some unrelated AABB performance testing and noticed stable sort being used in a couple places.

I don't have a great performance/memory profile to show a meaningful difference here, but there doesn't seem to be a reason we need to stable sort in these places.

(probably not worth a changelog entry)

adamreichold commented 4 months ago

(probably not worth a changelog entry)

Actually, I think an entry would nice: Better performance is always something downstreams will look forward to and if our reasoning is wrong, the entry will give downstreams at least hint at how we broke their programs.

urschrei commented 4 months ago

(probably not worth a changelog entry)

Actually, I think an entry would nice: Better performance is always something downstreams will look forward to and if our reasoning is wrong, the entry will give downstreams at least hint at how we broke their programs.

I agree; this is exactly the sort of change that belongs in the changelog!

grovesNL commented 4 months ago

Sounds good! Just added a changelog entry.