Implement DeepMap using a combined hash

[peterm-canva]

Draft for now

[taj-p]

How are our strongHashes disposed of? Should we delete them from this.state.strongHashes here somehow?

[peterm-canva]

I think we have to leak them unfortunately.

Registered symbols are never collected, hence not allowed in a WeakMap, hence no way to trigger something on their disposal. I'm not too worried about those as I guess they are very rare.

Regular symbols on Firefox is a bigger deal. We can't just delete from this.state.strongHashes in delete() though, because the same symbol hash might have been used by multiple cached calls to our function. We would have to store a reference count in strongHashes and update it.

I don't see any technical blocker, but not sure if it's worth it just for Firefox

[taj-p]

Question: Does this requirement still hold for a flat map approach? Can we now allow safe concurrent modification?

[peterm-canva]

True, that's an advantage. If I proceed with this I'll change that

[taj-p]

What are the owners thoughts of simplifying DeepMap and removing DeepMapEntry? I notice that most of its API is unused by computedFn

// Delete DeepMapEntry

export class DeepMap<T extends object> {
  private readonly store = new Map<string, T>();
  private readonly weakHashes = new WeakMap<object | Function | Symbol, string>();
  private readonly strongHashes = new Map<Symbol, string>();
  private hashId = 0;
  private argsLength = -1

  get(args: Array<Readonly<unknown>>): { entry: T | undefined, setEntry: (val: T) => void, deleteEntry: () => void } {
    if (this.argsLength === -1) this.argsLength = args.length
    else if (this.argsLength !== args.length)
      throw new Error(
        `DeepMap should be used with functions with a consistent length, expected: ${this.argsLength}, got: ${args.length}`
      )

    const entryKey = args.map((arg) => this.prehash(arg)).join("")
    let deleted = false;
    return {
      entry: this.store.get(entryKey),
      setEntry: (val: T) => {
        if (!deleted) this.store.set(entryKey, val)
        else throw new Error('entry is disposed')
      },
      deleteEntry: () => {
        this.store.delete(entryKey);
        deleted = true;
      },
    }
  }

  private prehash(arg: Readonly<unknown>): string | number {
    ...
  }
}

[taj-p]

Please resolve @peterm-canva if out of scope!! 🙏

[peterm-canva]

Yep it could definitely be considered, I wasn't sure of the reason for the current API. I kept this change to the same API for now so I could write tests that showed the new impl was equivalent

[taj-p]

An alternate method could be to detach them from the instance and use module scoped maps instead. This will reduce the memory requirements of each DeepMap instance and may afford de-duplication efficiency if two computedFns reference use the same object argument (I believe we do this quite often at Canva).

const weakHashes = new WeakMap<object | Function | Symbol, string>();
const strongHashes = new Map<Symbol, string>();

function prehash(...) { ... }
function getHash(...) { ... }