Skip to content

Commit 3d15697

Browse files
committed
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.
1 parent 19f5f8c commit 3d15697

File tree

2 files changed

+36
-10
lines changed

2 files changed

+36
-10
lines changed

CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,11 @@
2222
- Node.js using the improved browser algorithm can be ~4-9% faster overall.
2323
- Node.js native `Buffer` conversion can be ~5-12% faster overall.
2424
- Optimize a N-Quads serialization call.
25+
- Optimize the `IdentifierIssuer` `existing` `Map`:
26+
- Change clones from always-copy to copy-on-write.
27+
- Depending on shape of data, this can reduce `Map` copies by ~90%. However,
28+
in some data patterns, these will have very few entries, so overall
29+
performance may not be noticeably effected.
2530

2631
### Fixed
2732
- Disable native lib tests in a browser.

lib/IdentifierIssuer.js

Lines changed: 31 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright (c) 2016-2021 Digital Bazaar, Inc. All rights reserved.
2+
* Copyright (c) 2016-2023 Digital Bazaar, Inc. All rights reserved.
33
*/
44
'use strict';
55

@@ -9,12 +9,14 @@ module.exports = class IdentifierIssuer {
99
* identifiers, keeping track of any previously issued identifiers.
1010
*
1111
* @param prefix the prefix to use ('<prefix><counter>').
12-
* @param existing an existing Map to use.
12+
* @param existing an existing refs and Map object to use.
1313
* @param counter the counter to use.
1414
*/
15-
constructor(prefix, existing = new Map(), counter = 0) {
15+
constructor(prefix, existing = {refs: 0, map: new Map()}, counter = 0) {
1616
this.prefix = prefix;
17-
this._existing = existing;
17+
this.existing = existing;
18+
// add ref to shared map
19+
this.existing.refs++;
1820
this.counter = counter;
1921
}
2022

@@ -24,8 +26,8 @@ module.exports = class IdentifierIssuer {
2426
* @return a copy of this IdentifierIssuer.
2527
*/
2628
clone() {
27-
const {prefix, _existing, counter} = this;
28-
return new IdentifierIssuer(prefix, new Map(_existing), counter);
29+
const {prefix, existing, counter} = this;
30+
return new IdentifierIssuer(prefix, existing, counter);
2931
}
3032

3133
/**
@@ -38,7 +40,7 @@ module.exports = class IdentifierIssuer {
3840
*/
3941
getId(old) {
4042
// return existing old identifier
41-
const existing = old && this._existing.get(old);
43+
const existing = old && this.existing.map.get(old);
4244
if(existing) {
4345
return existing;
4446
}
@@ -49,7 +51,26 @@ module.exports = class IdentifierIssuer {
4951

5052
// save mapping
5153
if(old) {
52-
this._existing.set(old, identifier);
54+
if(this.existing.refs > 1) {
55+
// copy-on-write shared map
56+
// TODO: improve copy-on-write reference handling
57+
// - current code handles copying the 'existing' maps when it is
58+
// shared
59+
// - it will remove a reference when doing a copy
60+
// - a reference is NOT removed when a copy is no longer used
61+
// - need a `release()` call or similar to do this and add it
62+
// throughout the code as needed
63+
// - this won't result in errors, only extra copies if a child does
64+
// not do an update, is done, and a parent then does an update
65+
// unref shared map
66+
this.existing.refs--;
67+
// copy to new map
68+
this.existing = {
69+
refs: 1,
70+
map: new Map(this.existing.map)
71+
};
72+
}
73+
this.existing.map.set(old, identifier);
5374
}
5475

5576
return identifier;
@@ -65,7 +86,7 @@ module.exports = class IdentifierIssuer {
6586
* false if not.
6687
*/
6788
hasId(old) {
68-
return this._existing.has(old);
89+
return this.existing.map.has(old);
6990
}
7091

7192
/**
@@ -75,6 +96,6 @@ module.exports = class IdentifierIssuer {
7596
* @return the list of old IDs that has been issued new IDs in order.
7697
*/
7798
getOldIds() {
78-
return [...this._existing.keys()];
99+
return [...this.existing.map.keys()];
79100
}
80101
};

0 commit comments

Comments
 (0)