HashMap performance fix.

[matthewhammer]

Avoids rehashing when the hash table grows. Fixes one known performance issue.

Background

There are several performance issues with the HashMap that Motoko devs experience when they insert many keys and the table's underlying array re-grows.

Like a usual hash table, each growth step adds an O(n) overhead to what would otherwise be an O(1) insertion operation. But what's worse here, the current HashMap (before this PR) actually re-runs the hash function on every key. For Text keys with even moderate sizes, this has been shown to be prohibitive even at moderate map sizes.

The issue is critical, as it manifests as a failure to insert more keys, as regrowth goes beyond the current message cycle limit. Even with this limit relaxed or removed in the future, it would ideal to avoid this rehashing step. This PR does that.

[ggreif]

Is this backward-compatible with the stable vars that are already externalised? @matthewhammer

[matthewhammer]

Is this backward-compatible with the stable vars that are already externalised? @matthewhammer

No stable vars will use this type, since HashMap is a class, and not available to be mentioned there. So trivially, yes, this will preserve the types of all stable vars.

Now that you mention it, I think this PR could be considered "improvements only" and should not break anything running today. (Since the class API is not changing, any code using the class remains fine too.)

[crusso]

I'd just use a more efficient type for the internal key type definition.

[matthewhammer]

@crusso PTAL.

[crusso]

LGTM!

[ByronBecker]

Is this backward-compatible with the stable vars that are already externalised? @matthewhammer

No stable vars will use this type, since HashMap is a class, and not available to be mentioned there. So trivially, yes, this will preserve the types of all stable vars.

Now that you mention it, I think this PR could be considered "improvements only" and should not break anything running today. (Since the class API is not changing, any code using the class remains fine too.)

@matthewhammer @crusso The interface is compatible, but what about the data running underneath the change? I'm thinking specifically for users that are using the system pre/post upgrade functions to serialize/deserialize their HashMap to and from stable memory during upgrades.

Without a specific postUpgrade() that includes the extra key hash step, I'm just trying to understand how this would work.

My naive understanding.

1. Application is running using old version of HashMap (k is just the key)
2. Developer changes code locally to use package set or release of motoko-base (unknowingly picks up this new version). Let's say they were using a preUpgrade/postUpgrade to hold the internals of the HashTable class, like:

  table: Hashtable<K, V>;
  _count: Nat;

3. Developer assumes that nothing happened with how the data is stored in the HashMap data structure, and pushes out a dfx canister install <canister_name> -m upgrade.
4. At this point, the upgrade should throw some sort of warning stating that the underlying types have changed

// Old
type KVs<K, V> = AssocList.AssocList<K, V>;

// New
type KVs<K, V> = AssocList.AssocList<Key<K>, V>;

5. Developer in their postUpgrade() now has to reconstruct the hashes for each individual key to match the new type signature.

6. After making this postUpgrade() addition, the developer again tries the upgrade, but this time continues through the warning (The upgrade either succeeds, or fails due to hitting an upgrade limit b/c of re-hashing every single key - what happens if this is the case?).

7. The developer then removes the rehashing portion of postUpgrade() method since this is no longer necessary and pushes out an additional upgrade (no warnings).

Usually for breaking changes, libraries provide migration notes - so It might be nice in the documentation to include how one can upgrade from the old version of HashMap to the new version (i.e. include the recommended code for the postUpgrade() code for adapting the underlying table in HashTable).