Optimise dict equality check on JavaScript

[richard-viney]

The dict equality check now returns false as soon as any item doesn't match, rather than continuing to unnecessarily check remaining items.

[richard-viney]

I'll look into adding ES6-style iterators and see what the performance is like.

[richard-viney]

The fastest ES6 iterator I came up with iterates at about half the speed of the forEach() approach. Maintaining the iterator state is slightly involved, and a few extra temporary objects get created, which is probably what makes it slower.

Here's the code:

/**
 * @template K,V
 * @param {undefined | Node<K,V>} root
 * @returns {IterableIterator<[K, V]>}
 */
function iterator(root) {
  // Return an empty iterator if there's no root node
  if (root === undefined) {
    return [][Symbol.iterator]();
  }

  // Stack of nodes in the trie that tracks the iterator's current position
  const nodes = [[root?.array, 0]];

  // Quick access to the current node being iterated
  let currentNodeItems = nodes[0][0];
  let currentNodeItemIndex = nodes[0][1];

  return {
    next() {
      while (true) {
        // If the current node is fully iterated then pop it off the stack
        if (currentNodeItemIndex >= currentNodeItems.length) {
          nodes.pop();

          // Finish when the final node is popped off
          if (nodes.length === 0) {
            return { done: true };
          }

          currentNodeItems = nodes[nodes.length - 1][0];
          currentNodeItemIndex = nodes[nodes.length - 1][1];
        } else {
          const item = currentNodeItems[currentNodeItemIndex];

          // Move the current node to its next item
          currentNodeItemIndex++;

          if (item === undefined) {
            continue;
          } else if (item.type === ENTRY) {
            return { value: [item.k, item.v], done: false };
          } else {
            nodes[nodes.length - 1][1] = currentNodeItemIndex;

            // Iterate into this child node
            nodes.push([item.array, 0]);

            currentNodeItems = item.array;
            currentNodeItemIndex = 0;
          }
        }
      }
    },

    [Symbol.iterator]() {
      return this;
    },
  };
}

It may be hard/impossible to match the speed of forEach() with an ES6-style iterator here.

I don't know of any 3rd-party uses of Dict.forEach() in JS FFI code that currently return false from the callback function and therefore would be broken by this change, but it can't be definitively ruled out. Should we make it a separate function?

[lpil]

I agree, that's too much of a performance hit.

Returning false is not an option. How about we use throw- that's a procedural way to break out of a loop and it infers no runtime cost for the other uses.

[richard-viney]

Great idea, I've switched to throwing a unique symbol on inequality and propagating any other exceptions that may occur.

Note that the prelude is currently silently swallowing all exceptions from an equals() call, so any other exceptions will never be seen:

Do you recall why the prelude does that?

[lpil]

Ah right. It's because == should never throw, and we're calling any function called equals, but we don't know what that might do.

The code looks good to me, it is ready to merge? Or does that catch cause a problem?

[richard-viney]

Ready to merge 👍

[lpil]

Thank you!!