From 3d15697e49a06a20a6ab866c670ed878fdbc3c97 Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Thu, 30 Mar 2023 20:25:21 -0400 Subject: [PATCH 1/2] Optimize 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. --- CHANGELOG.md | 5 +++++ lib/IdentifierIssuer.js | 41 +++++++++++++++++++++++++++++++---------- 2 files changed, 36 insertions(+), 10 deletions(-) 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..e2e01bd 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; + this.existing = existing; + // add ref to shared map + this.existing.refs++; this.counter = counter; } @@ -24,8 +26,8 @@ module.exports = class IdentifierIssuer { * @return a copy of this IdentifierIssuer. */ clone() { - const {prefix, _existing, counter} = this; - return new IdentifierIssuer(prefix, new Map(_existing), counter); + const {prefix, existing, counter} = this; + 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()]; } }; From 1e25326d81e294922cb7b68c9cb59801260611ae Mon Sep 17 00:00:00 2001 From: "David I. Lehn" Date: Fri, 31 Mar 2023 14:50:25 -0400 Subject: [PATCH 2/2] Prefix internal property. --- lib/IdentifierIssuer.js | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/IdentifierIssuer.js b/lib/IdentifierIssuer.js index e2e01bd..5c32424 100644 --- a/lib/IdentifierIssuer.js +++ b/lib/IdentifierIssuer.js @@ -14,9 +14,9 @@ module.exports = class IdentifierIssuer { */ constructor(prefix, existing = {refs: 0, map: new Map()}, counter = 0) { this.prefix = prefix; - this.existing = existing; + this._existing = existing; // add ref to shared map - this.existing.refs++; + this._existing.refs++; this.counter = counter; } @@ -26,8 +26,8 @@ module.exports = class IdentifierIssuer { * @return a copy of this IdentifierIssuer. */ clone() { - const {prefix, existing, counter} = this; - return new IdentifierIssuer(prefix, existing, counter); + const {prefix, _existing, counter} = this; + return new IdentifierIssuer(prefix, _existing, counter); } /** @@ -40,7 +40,7 @@ module.exports = class IdentifierIssuer { */ getId(old) { // return existing old identifier - const existing = old && this.existing.map.get(old); + const existing = old && this._existing.map.get(old); if(existing) { return existing; } @@ -51,7 +51,7 @@ module.exports = class IdentifierIssuer { // save mapping if(old) { - if(this.existing.refs > 1) { + 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 @@ -63,14 +63,14 @@ module.exports = class IdentifierIssuer { // - 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--; + this._existing.refs--; // copy to new map - this.existing = { + this._existing = { refs: 1, - map: new Map(this.existing.map) + map: new Map(this._existing.map) }; } - this.existing.map.set(old, identifier); + this._existing.map.set(old, identifier); } return identifier; @@ -86,7 +86,7 @@ module.exports = class IdentifierIssuer { * false if not. */ hasId(old) { - return this.existing.map.has(old); + return this._existing.map.has(old); } /** @@ -96,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.map.keys()]; + return [...this._existing.map.keys()]; } };