Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reduce unnecessary Comparators in Range set #324

Conversation

isaacs
Copy link
Contributor

@isaacs isaacs commented Apr 20, 2020

There are a few cases that lead to Range's Comparator sets being longer
than strictly necessary. This reduces performance of methods that
iterate over ranges repeatedly (for example, intersects and subset),
and leads to some confusing toString output like turning x || * || X
into |||| instead of *.

  • If any simple range in the set contains the null set <0.0.0-0, then
    the entire simple range is the null set. 2.x <0.0.0-0 is the same as
    just <0.0.0-0. (This is used for >* and <*, which cannot match
    anything.)
  • Ensure that a given Comparator will only occur once within each simple
    range set. 2.3.x ^2.3 doesn't need to include >=2.3.0 more than
    once.
  • If a simple range set contains more than one comparator, remove any *
    comparators. * >=2.3.4 is the same as just >=2.3.4. This was
    already being done in the cast to a string, but some ANY Comparators
    would be left behind in the set used for matching.
  • If a Range set contains the simple range *, then drop any other
    simple ranges in the set. * || 2.x is the same as *.

There's still some unnecessary comparators in there. For example, the
range 2.3 ^2.3.4 parses to >=2.3.0 <2.4.0-0 >=2.3.4 <3.0.0-0. Of
course, anything that is <2.4.0-0 is also <3.0.0-0, and anything
that is >=2.3.4 is also >=2.3.0, so the <3.0.0-0 and >=2.3.0
Comparators are not necessary. But simplifying those out would be a bit
more work.

To do that, we could walk the set of Comparators checking to see if they
are a subset of any other Comparators in the list, and if so, removing
them. The subset check would not have to be a full Range.subset(); we
could just see if the gtlt points in the same direction, and if one
semver is greater than the other. It's an O(n^2) operation, but one on
typically very small n.

@isaacs isaacs force-pushed the isaacs/optimization-reduce-unnecessary-comparators-in-range-set branch from 48e4cc6 to 499284b Compare April 20, 2020 22:13
There are a few cases that lead to Range's Comparator sets being longer
than strictly necessary.  This reduces performance of methods that
iterate over ranges repeatedly (for example, `intersects` and `subset`),
and leads to some confusing toString output like turning `x || * || X`
into `||||` instead of `*`.

- If any simple range in the set contains the null set <0.0.0-0, then
  the entire simple range is the null set. `2.x <0.0.0-0` is the same as
  just `<0.0.0-0`.  (This is used for `>*` and `<*`, which cannot match
  anything.)
- Ensure that a given Comparator will only occur once within each simple
  range set.  `2.3.x ^2.3` doesn't need to include `>=2.3.0` more than
  once.
- If a simple range set contains more than one comparator, remove any `*`
  comparators.  `* >=2.3.4` is the same as just `>=2.3.4`.  This was
  already being done in the cast to a string, but some `ANY` Comparators
  would be left behind in the set used for matching.
- If a Range set contains the simple range `*`, then drop any other
  simple ranges in the set. `* || 2.x` is the same as `*`.

There's still some unnecessary comparators in there.  For example, the
range `2.3 ^2.3.4` parses to `>=2.3.0 <2.4.0-0 >=2.3.4 <3.0.0-0`.  Of
course, anything that is `<2.4.0-0` is also `<3.0.0-0`, and anything
that is `>=2.3.4` is also `>=2.3.0`, so the `<3.0.0-0` and `>=2.3.0`
Comparators are not necessary.  But simplifying those out would be a bit
more work.

To do that, we could walk the set of Comparators checking to see if they
are a subset of any other Comparators in the list, and if so, removing
them.  The subset check would not have to be a full Range.subset(); we
could just see if the gtlt points in the same direction, and if one
semver is greater than the other.  It's an O(n^2) operation, but one on
typically very small n.
@coveralls
Copy link

coveralls commented Apr 20, 2020

Coverage Status

Coverage remained the same at 100.0% when pulling 028a2f7 on isaacs/optimization-reduce-unnecessary-comparators-in-range-set into 226e6dc on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 499284b on isaacs/optimization-reduce-unnecessary-comparators-in-range-set into 226e6dc on master.

@isaacs isaacs force-pushed the isaacs/optimization-reduce-unnecessary-comparators-in-range-set branch from 499284b to 028a2f7 Compare April 20, 2020 22:16
@isaacs isaacs closed this in bcab95a May 8, 2020
kellyselden pushed a commit to kellyselden/monorepo-next that referenced this pull request Dec 22, 2020
@lukekarrys lukekarrys deleted the isaacs/optimization-reduce-unnecessary-comparators-in-range-set branch February 24, 2022 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants