Did removing `selector-set` have a adverse effect on performance. about remote-form HOT 5 OPEN

// Naive loop
// O(n x m) -> at a tree depth of 20 and selector list of 100
// this is 2,000 matches calls.
for (const el of elements) {
  for (const sel of selectors) {
    if (el.matches(sel)) {
      // do something
    }
  }
}

// SelectorSet
for (const el of elements) {
  // set calls matches only against selectors that have a chance of
  // matching, dropping 2,000 calls to just a handful.
  if (set.matches(el)) {
      // do something
    }
  }
}

Prior art

git clone https://github.com/dgraham/delegated-events.git
cd delegated-events
npm install
npm run bootstrap
npm run bench

from remote-form.

You also need to run npm run bootstrap in order to pull the submodules containing the vendored libraries.

from remote-form.

I ended up taking the jsperf test linked on the selector-set README and adding a new case for the Map lookup and found it performs about the same as linear.

SelectorSet x 50,848 ops/sec ±0.87% (63 runs sampled)
Linear x 104 ops/sec ±2.31% (52 runs sampled)
Map x 122 ops/sec ±1.76% (51 runs sampled)
Fastest is SelectorSet

from remote-form.

I wanted to check what the difference this change has on code on github.com and added the following performance measuring to the getMatches call and ran some form submissions. I ran these on a pull request in my local environment where there are approximately 100 forms.

Note that all the results are in milliseconds

WeakMap

   function getMatches(el) {
+    const start = window.performance.now()
     const results = []
     for (const selector of formHandlers.keys()) {
       if (el.matches(selector)) {
         const handlers = formHandlers.get(selector) || []
         results.push(...handlers)
       }
     }
+    console.debug(`Time running: ${window.performance.now() - start}`)
     return results
   }

Time running: 0.17500005196779966
Time running: 0.1300000585615635
Time running: 0.11499994434416294

SelectorSet

+  const start = window.performance.now()
   const matches = selectorSet && selectorSet.matches(form)
+  console.debug(`Time running: ${window.performance.now() - start}`)

Time running: 0.04000007174909115
Time running: 0.06000010762363672
Time running: 0.04000007174909115

So it seems in practice the current method is double the time SelectorSet takes to run the same operation but the difference is ~0.1 milliseconds. It might be worth being as performant as we can here but I would lean towards having less dependencies in remote-form.

selector-set is 1.5kB which takes 30ms to download on 3G according to Bundle Phobia.

from remote-form.

@koddsson I just stumbled on this. Sounds like the perf differences were negligible. Good to close?

from remote-form.