Skip to content

Commit

Permalink
fix: improve addChild and path performance (#1891)
Browse files Browse the repository at this point in the history
Improve the performance of the `constructs` library by removing several branches in the library's hot paths and switching to some more performant operations.

On my laptop, the benchmark I used (shared in a gist [here](https://gist.github.com/Chriscbr/fa69e5f7f35d5aa4dc312dd7025952b1)) takes about 5.0-5.3 seconds with the old version of constructs, and about 1.1-1.2 seconds with the new version of constructs.

Misc:
- Removed an unused file in the `test` folder.
- Added some files to `npmignore` to reduce the package's npm footprint.
  • Loading branch information
Chriscbr authored Aug 29, 2023
1 parent f47fb2f commit fdde3da
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 81 deletions.
2 changes: 2 additions & 0 deletions .npmignore

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions .projenrc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,5 +81,6 @@ project.buildWorkflow?.addPostBuildJobCommands(
{ checkoutRepo: true },
);

project.npmignore?.exclude('/scripts/', '.projenrc.ts');

project.synth();
17 changes: 7 additions & 10 deletions src/construct.ts
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,12 @@ export class Node {
* Components are separated by '/'.
*/
public get path(): string {
const components = this.scopes.filter(c => c.node.id).map(c => c.node.id);
const components = [];
for (const scope of this.scopes) {
if (scope.node.id) {
components.push(scope.node.id);
}
}
return components.join(Node.PATH_SEP);
}

Expand Down Expand Up @@ -417,21 +422,13 @@ export class Node {
throw new Error(`Cannot add children to "${this.path}" during synthesis`);
}

if (childName in this._children) {
if (this._children[childName]) {
const name = this.id ?? '';
const typeName = this.host.constructor.name;
throw new Error(`There is already a Construct with name '${childName}' in ${typeName}${name.length > 0 ? ' [' + name + ']' : ''}`);
}

if (!childName && this.id) {
throw new Error(`cannot add a nameless construct to the named scope: ${this.path}`);
}

this._children[childName] = child;

if (Object.keys(this._children).length > 1 && Object.keys(this._children).filter(x => !x).length > 0) {
throw new Error('only a single construct is allowed in a scope if it has an empty name');
}
}
}

Expand Down
71 changes: 0 additions & 71 deletions test/evaluate-cfn.ts

This file was deleted.

0 comments on commit fdde3da

Please sign in to comment.