diff --git a/CHANGELOG.md b/CHANGELOG.md index a8ffb68..3b50275 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,11 @@ - Node.js using the improved browser algorithm can be ~4-9% faster overall. - Node.js native `Buffer` conversion can be ~5-12% faster overall. - Optimize a N-Quads serialization call. +- Optimize the `IdentifierIssuer` `existing` `Map`: + - Change clones from always-copy to copy-on-write. + - Depending on shape of data, this can reduce `Map` copies by ~90%. However, + in some data patterns, these will have very few entries, so overall + performance may not be noticeably effected. ### Fixed - Disable native lib tests in a browser. diff --git a/lib/IdentifierIssuer.js b/lib/IdentifierIssuer.js index aad41b2..5c32424 100644 --- a/lib/IdentifierIssuer.js +++ b/lib/IdentifierIssuer.js @@ -1,5 +1,5 @@ /* - * Copyright (c) 2016-2021 Digital Bazaar, Inc. All rights reserved. + * Copyright (c) 2016-2023 Digital Bazaar, Inc. All rights reserved. */ 'use strict'; @@ -9,12 +9,14 @@ module.exports = class IdentifierIssuer { * identifiers, keeping track of any previously issued identifiers. * * @param prefix the prefix to use (''). - * @param existing an existing Map to use. + * @param existing an existing refs and Map object to use. * @param counter the counter to use. */ - constructor(prefix, existing = new Map(), counter = 0) { + constructor(prefix, existing = {refs: 0, map: new Map()}, counter = 0) { this.prefix = prefix; this._existing = existing; + // add ref to shared map + this._existing.refs++; this.counter = counter; } @@ -25,7 +27,7 @@ module.exports = class IdentifierIssuer { */ clone() { const {prefix, _existing, counter} = this; - return new IdentifierIssuer(prefix, new Map(_existing), counter); + return new IdentifierIssuer(prefix, _existing, counter); } /** @@ -38,7 +40,7 @@ module.exports = class IdentifierIssuer { */ getId(old) { // return existing old identifier - const existing = old && this._existing.get(old); + const existing = old && this._existing.map.get(old); if(existing) { return existing; } @@ -49,7 +51,26 @@ module.exports = class IdentifierIssuer { // save mapping if(old) { - this._existing.set(old, identifier); + if(this._existing.refs > 1) { + // copy-on-write shared map + // TODO: improve copy-on-write reference handling + // - current code handles copying the 'existing' maps when it is + // shared + // - it will remove a reference when doing a copy + // - a reference is NOT removed when a copy is no longer used + // - need a `release()` call or similar to do this and add it + // throughout the code as needed + // - this won't result in errors, only extra copies if a child does + // not do an update, is done, and a parent then does an update + // unref shared map + this._existing.refs--; + // copy to new map + this._existing = { + refs: 1, + map: new Map(this._existing.map) + }; + } + this._existing.map.set(old, identifier); } return identifier; @@ -65,7 +86,7 @@ module.exports = class IdentifierIssuer { * false if not. */ hasId(old) { - return this._existing.has(old); + return this._existing.map.has(old); } /** @@ -75,6 +96,6 @@ module.exports = class IdentifierIssuer { * @return the list of old IDs that has been issued new IDs in order. */ getOldIds() { - return [...this._existing.keys()]; + return [...this._existing.map.keys()]; } };