Skip to content

Commit

Permalink
Review
Browse files Browse the repository at this point in the history
  • Loading branch information
konstin committed Nov 14, 2024
1 parent 292f3d0 commit cd57a69
Showing 1 changed file with 13 additions and 2 deletions.
15 changes: 13 additions & 2 deletions version-ranges/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -834,8 +834,16 @@ impl<V: Ord> FromIterator<(Bound<V>, Bound<V>)> for Ranges<V> {
fn from_iter<T: IntoIterator<Item = (Bound<V>, Bound<V>)>>(iter: T) -> Self {
// We have three constraints we need to fulfil:
// 1. The segments are sorted, from lowest to highest (through `Ord`): By sorting.
// 2. Each segment contains at least one version (start < end): By `union`.
// 3. There is at least one version between two segments: By `union`.
// 2. Each segment contains at least one version (start < end): By skipping invalid
// segments.
// 3. There is at least one version between two segments: By merging overlapping elements.
//
// Technically, the implementation has a O(n²) worst case complexity since we're inserting
// and removing. This has two motivations: One is that we don't have any performance
// critical usages of this method as of this writing, so we have no real world benchmark.
// The other is that we get the elements from an iterator, so to avoid moving elements
// around we would first need to build a different, sorted collection with extra
// allocation(s), before we could build our real segments. --Konsti

// For this implementation, we choose to only build a single smallvec and insert or remove
// in it, instead of e.g. collecting the segments into a sorted datastructure first and then
Expand Down Expand Up @@ -986,6 +994,9 @@ impl<V: Ord> FromIterator<(Bound<V>, Bound<V>)> for Ranges<V> {
// following: |------|
//
// final: |------| |------| |------|

// This line is O(n), which makes the algorithm O(n²), but it should be good
// enough for now.
segments.insert(insertion_point, segment);
}
}
Expand Down

0 comments on commit cd57a69

Please sign in to comment.