Perf: Clone only valid segments

[konstin]

Remove 30% of the runtime which was previously being spent cloning versions that we couldn't use anyway.

flamegraph before:

flamegraph after:

Notice how the ::cloned section disappears.

[Eh2406]

Amazing find! I look forward to your PR up streaming it :-P!
None of the upstream benchmarks properly capture the case when V.clone() has a cost (I should probably fix that), but I wouldn't be surprised if this improves even for the cheap case.

[konstin]

I have another optimization on top (daf6838), and i'm pretty sure we could push this even further, but this is just a stop gap measure against pubgrub-rs#135; We shouldn't need to compute that many intersections and there shouldn't be that many (or more than a few at all) segments in the range.

[Eh2406]

I like that change as well. Intersection is the workhorse of PubGrub, even if we fix the algorithmic problems behind pubgrub-rs#135 these improvements will still matter.

This work suggests that, as an additional stopgap, you might be served by using RC around your versions.

[Eh2406]

I got nerds not by this over the weekend. Here's what I've come up with pubgrub-rs@a358c39

The ideas are:

1. Like this PR, avoid cloning till the end
2. Instead of using prt::eq keep track of which iter to advance directly.
3. Because we know which end came from, and we know that for each iterator start < end, check for validity before determining start
4. Don't redundantly check e > i

My synthetic tests suggest that this would be a significant improvement beyond this PR, but would love to find out the real improvement.

[konstin]

Thank you, that looks like the optimal solution! I'm currently trying to minimize the number of segments based on pubgrub-rs#156, i'll report how this goes. Either way it would be great to merge pubgrub-rs@a358c39.

[Eh2406]

Sorry about hijacking your thread. Can you play with https://github.com/pubgrub-rs/pubgrub/pull/new/merge_dep it is an optimization for when the same dependency statement occurs on different versions of packages. It makes a big difference for synthetic benchmarks where it applies, I would love to know if it's relevant for your real-world cases.

[konstin]

Closing in favor of @Eh2406's better version