[WIP] Remove performance bottleneck in active element preservation

[botandrose]

It turns out that Element.contains is really slow, so we get a big perf win by precomputing the active element parent path and doing an array inclusion check instead.

I'm marking this PR as WIP because I want to add a benchmark for this before merging. None of the existing perf benchmark have active elements, so they don't really capture the slowdown. But considering how impactful this seems to be, a benchmark seems like a good idea. It's about an order of magnitude in my BARD Tracker app!

Funny enough, though, I'm already seeing perf increases of around 10-35% in the existing benchmarks, even though they don't contain any active elements!

Closes #134

[MichaelWest22]

Note that activeElement defaults to body so it will still be checking constantly if the element contains body element.

[botandrose]

@MichaelWest22 Ohh, you're right! That explains the perf win in the existing benchmarks then. Maybe we don't need a separate benchmark.

[MichaelWest22]

having a body exclude check would probably give a simpler perf boost. But this may not help when a real item is active and there could be a perf hit then. I wonder if contains could perform better if its not being passed a giant body tag??? That could be worth testing to confirm maybe

[botandrose]

@MichaelWest22 So maybe this could be optimized a bit more by checking if the active element is within the oldNode at startup, and only populating the array if it is. Then we only do the test if the array is nonempty.

BTW, I also looked into doing Set inclusion instead of Array inclusion, but it turns out that's only faster with much larger collection sizes. Array is faster for smaller collections.

[botandrose]

@MichaelWest22 I don't think that its more expensive to pass a body tag with a million children, than a leaf node with none, provided we're just testing for array inclusion. But this is just a hunch. In other words, DOM elements are passed by reference, not by value.

[MichaelWest22]

You could get your createActiveElementAndParents routine to have the while loop to run while elt !== oldNode so it will return the array only if its valid with active element in the oldNode. Then add a early return [] if elt is null inside the loop for the other case maybe

[botandrose]

@MichaelWest22 Good suggestions, I fiddled with them a bit and ended up with this:

       let activeElementAndParents = [];
       let elt = document.activeElement;
       if (elt?.tagName !== "BODY" && oldNode.contains(elt)) {
        while (elt) {
          activeElementAndParents.push(elt);
          if (elt === oldNode) break;
          elt = elt.parentElement;
        }
      }
      return activeElementAndParents;

Perf win is now 35% on the benchmark. This is about as good as I can imagine getting it, unless you have any more ideas?

[MichaelWest22]

    function createActiveElementAndParents(oldNode) {
      /** @type {Element[]} */
      let activeElementAndParents = [];
      let elt = document.activeElement;
      while (elt !== oldNode) {
        if (!elt) return []
        activeElementAndParents.push(elt);
        elt = elt.parentElement;
      }
      return activeElementAndParents;
    }

Yeah something like this could be slightly more efficient as it avoids that extra contains call even though it is only called once so I don't think you will notice any different in full run performance really.
Edit. But your version will be slightly faster for the body case anyway but a tiny bit slower for the other case. body parent is html which has a null parent so in the body case it is only two parent lookups

[botandrose]

@MichaelWest22 Yeah I'm okay with a single Element.contains call. The big performance win is not calling contains dozens or hundreds of time in the hot path. Your code looks good too. Super clever detecting hitting null vs hitting oldNode!

But yeah I don't think its so important that createActiveElementAndParents is super optimized, since we're just calling it once on startup. The real win is when findBestMatch runs faster as a result.