Memoize range parsing

This yields about a 10-20% speed improvement to a lot of common npm
operations that spend most of their CPU time parsing a relatively small
set of dependency ranges.

It does mean that one `range.set` may contain the same Comparator
objects as another range with the same sub-range.  That was technically
already possible, due to the `Comparator.ANY` magic range, but it did
require a bit of tweaking in the `subset` function, which is also now
a bit faster when calculating subsets for ranges that are partly or
entirely identical.

classes/comparator.js

@@ -5,12 +5,7 @@ class Comparator {
     return ANY
   }
   constructor (comp, options) {
-    if (!options || typeof options !== 'object') {
-      options = {
-        loose: !!options,
-        includePrerelease: false
-      }
-    }
+    options = parseOptions(options)
 
     if (comp instanceof Comparator) {
       if (comp.loose === !!options.loose) {
@@ -132,6 +127,7 @@ class Comparator {
 
 module.exports = Comparator
 
+const parseOptions = require('../internal/parse-options')
 const {re, t} = require('../internal/re')
 const cmp = require('../functions/cmp')
 const debug = require('../internal/debug')

classes/range.js

@@ -1,12 +1,7 @@
 // hoisted class for cyclic dependency
 class Range {
   constructor (range, options) {
-    if (!options || typeof options !== 'object') {
-      options = {
-        loose: !!options,
-        includePrerelease: false
-      }
-    }
+    options = parseOptions(options)
 
     if (range instanceof Range) {
       if (
@@ -82,8 +77,17 @@ class Range {
   }
 
   parseRange (range) {
-    const loose = this.options.loose
     range = range.trim()
+
+    // memoize range parsing for performance.
+    // this is a very hot path, and fully deterministic.
+    const memoOpts = Object.keys(this.options).join(',')
+    const memoKey = `parseRange:${memoOpts}:${range}`
+    const cached = cache.get(memoKey)
+    if (cached)
+      return cached
+
+    const loose = this.options.loose
     // `1.2.3 - 1.2.4` => `>=1.2.3 <=1.2.4`
     const hr = loose ? re[t.HYPHENRANGELOOSE] : re[t.HYPHENRANGE]
     range = range.replace(hr, hyphenReplace(this.options.includePrerelease))
@@ -129,7 +133,9 @@ class Range {
     if (rangeMap.size > 1 && rangeMap.has(''))
       rangeMap.delete('')
 
-    return [...rangeMap.values()]
+    const result = [...rangeMap.values()]
+    cache.set(memoKey, result)
+    return result
   }
 
   intersects (range, options) {
@@ -178,6 +184,10 @@ class Range {
 }
 module.exports = Range
 
+const LRU = require('lru-cache')
+const cache = new LRU({ max: 1000 })
+
+const parseOptions = require('../internal/parse-options')
 const Comparator = require('./comparator')
 const debug = require('../internal/debug')
 const SemVer = require('./semver')

classes/semver.js

@@ -2,15 +2,12 @@ const debug = require('../internal/debug')
 const { MAX_LENGTH, MAX_SAFE_INTEGER } = require('../internal/constants')
 const { re, t } = require('../internal/re')
 
+const parseOptions = require('../internal/parse-options')
 const { compareIdentifiers } = require('../internal/identifiers')
 class SemVer {
   constructor (version, options) {
-    if (!options || typeof options !== 'object') {
-      options = {
-        loose: !!options,
-        includePrerelease: false
-      }
-    }
+    options = parseOptions(options)
+
     if (version instanceof SemVer) {
       if (version.loose === !!options.loose &&
           version.includePrerelease === !!options.includePrerelease) {

functions/parse.js

@@ -2,13 +2,9 @@ const {MAX_LENGTH} = require('../internal/constants')
 const { re, t } = require('../internal/re')
 const SemVer = require('../classes/semver')
 
+const parseOptions = require('../internal/parse-options')
 const parse = (version, options) => {
-  if (!options || typeof options !== 'object') {
-    options = {
-      loose: !!options,
-      includePrerelease: false
-    }
-  }
+  options = parseOptions(options)
 
   if (version instanceof SemVer) {
     return version

internal/parse-options.js

@@ -0,0 +1,11 @@
+// parse out just the options we care about so we always get a consistent
+// obj with keys in a consistent order.
+const opts = ['includePrerelease', 'loose', 'rtl']
+const parseOptions = options =>
+  !options ? {}
+  : typeof options !== 'object' ? { loose: true }
+  : opts.filter(k => options[k]).reduce((options, k) => {
+    options[k] = true
+    return options
+  }, {})
+module.exports = parseOptions

ranges/subset.js

@@ -21,15 +21,18 @@ const compare = require('../functions/compare.js')
 //   - If EQ satisfies every C, return true
 //   - Else return false
 // - If GT
-//   - If GT is lower than any > or >= comp in C, return false
+//   - If GT.semver is lower than any > or >= comp in C, return false
 //   - If GT is >=, and GT.semver does not satisfy every C, return false
 // - If LT
-//   - If LT.semver is greater than that of any > comp in C, return false
+//   - If LT.semver is greater than any < or <= comp in C, return false
 //   - If LT is <=, and LT.semver does not satisfy every C, return false
 // - If any C is a = range, and GT or LT are set, return false
 // - Else return true
 
 const subset = (sub, dom, options) => {
+  if (sub === dom)
+    return true
+
   sub = new Range(sub, options)
   dom = new Range(dom, options)
   let sawNonNull = false
@@ -52,6 +55,9 @@ const subset = (sub, dom, options) => {
 }
 
 const simpleSubset = (sub, dom, options) => {
+  if (sub === dom)
+    return true
+
   if (sub.length === 1 && sub[0].semver === ANY)
     return dom.length === 1 && dom[0].semver === ANY
 
@@ -90,6 +96,7 @@ const simpleSubset = (sub, dom, options) => {
       if (!satisfies(eq, String(c), options))
         return false
     }
+
     return true
   }
 
@@ -101,15 +108,15 @@ const simpleSubset = (sub, dom, options) => {
     if (gt) {
       if (c.operator === '>' || c.operator === '>=') {
         higher = higherGT(gt, c, options)
-        if (higher === c)
+        if (higher === c && higher !== gt)
           return false
       } else if (gt.operator === '>=' && !satisfies(gt.semver, String(c), options))
         return false
     }
     if (lt) {
       if (c.operator === '<' || c.operator === '<=') {
         lower = lowerLT(lt, c, options)
-        if (lower === c)
+        if (lower === c && lower !== lt)
           return false
       } else if (lt.operator === '<=' && !satisfies(lt.semver, String(c), options))
         return false

test/internal/parse-options.js

@@ -0,0 +1,32 @@
+const t = require('tap')
+const parseOptions = require('../../internal/parse-options.js')
+
+t.test('falsey values always empty options object', t => {
+  t.strictSame(parseOptions(null), {})
+  t.strictSame(parseOptions(false), {})
+  t.strictSame(parseOptions(undefined), {})
+  t.strictSame(parseOptions(), {})
+  t.strictSame(parseOptions(0), {})
+  t.strictSame(parseOptions(''), {})
+  t.end()
+})
+
+t.test('truthy non-objects always loose mode, for backwards comp', t => {
+  t.strictSame(parseOptions('hello'), {loose: true})
+  t.strictSame(parseOptions(true), {loose: true})
+  t.strictSame(parseOptions(1), {loose: true})
+  t.end()
+})
+
+
+t.test('objects only include truthy flags we know about, set to true', t => {
+  t.strictSame(parseOptions(/asdf/), {})
+  t.strictSame(parseOptions(new Error('hello')), {})
+  t.strictSame(parseOptions({loose: true,a:1,rtl:false}), { loose: true })
+  t.strictSame(parseOptions({loose: 1,rtl:2,includePrerelease:10}), {
+    loose: true,
+    rtl: true,
+    includePrerelease: true,
+  })
+  t.end()
+})

test/ranges/subset.js

@@ -1,10 +1,14 @@
 const t = require('tap')
 const subset = require('../../ranges/subset.js')
+const Range = require('../../classes/range')
 
 // sub, dom, expect, [options]
 const cases = [
   ['1.2.3', '1.2.3', true],
+  ['1.2.3', '1.x', true],
   ['1.2.3 1.2.4', '1.2.3', true],
+  ['1.2.3 1.2.4', '1.2.9', true], // null set is subset of everything
+  ['1.2.3', '>1.2.0', true],
   ['1.2.3 2.3.4 || 2.3.4', '3', false],
   ['^1.2.3-pre.0', '1.x', false],
   ['^1.2.3-pre.0', '1.x', true, { includePrerelease: true }],
@@ -65,11 +69,45 @@ const cases = [
   ['<=3 <=2 <=1', '<4', true],
   ['>=1 >=2 >=3', '>0', true],
   ['>=3 >=2 >=1', '>0', true],
+  ['>=3 >=2 >=1', '>=3 >=2 >=1', true],
+  ['>2.0.0', '>=2.0.0', true],
 ]
 
-t.plan(cases.length)
+
+t.plan(cases.length + 1)
 cases.forEach(([sub, dom, expect, options = {}]) => {
   const msg = `${sub || "''"} ⊂ ${dom || "''"} = ${expect}` +
     (options ? ' ' + Object.keys(options).join(',') : '')
   t.equal(subset(sub, dom, options), expect, msg)
 })
+
+t.test('range should be subset of itself in obj or string mode', t => {
+  const range = '^1'
+  t.equal(subset(range, range), true)
+  t.equal(subset(range, new Range(range)), true)
+  t.equal(subset(new Range(range), range), true)
+  t.equal(subset(new Range(range), new Range(range)), true)
+
+  // test with using the same actual object
+  const r = new Range(range)
+  t.equal(subset(r, r), true)
+
+  // different range object with same set array
+  const r2 = new Range(range)
+  r2.set = r.set
+  t.equal(subset(r2, r), true)
+  t.equal(subset(r, r2), true)
+
+  // different range with set with same simple set arrays
+  const r3 = new Range(range)
+  r3.set = [...r.set]
+  t.equal(subset(r3, r), true)
+  t.equal(subset(r, r3), true)
+
+  // different range with set with simple sets with same comp objects
+  const r4 = new Range(range)
+  r4.set = r.set.map(s => [...s])
+  t.equal(subset(r4, r), true)
+  t.equal(subset(r, r4), true)
+  t.end()
+})