Skip to content

Commit

Permalink
fix(core): performance improvements (#1750)
Browse files Browse the repository at this point in the history
fix(core): performance improvements

Fix several performance issues:

- Only prepare the construct tree once.
- Don't regex-quote a constant string for every Token, do that only
  once.
- Replace JavaScript object with Map<> for dictionary use.
- Make sure we don't recurse twice into the arguments of `ignoreEmpty` during
  rendering of resources.

The latter three bring down synthesis time from ~3.5 seconds to ~1.3s
on an application with 100+ stacks and 6000+ constructs.
  • Loading branch information
rix0rrr authored Feb 13, 2019
1 parent 8713ac6 commit 77b516f
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 47 deletions.
12 changes: 11 additions & 1 deletion packages/@aws-cdk/cdk/lib/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ import { IConstruct, MetadataEntry, PATH_SEP, Root } from './core/construct';
* Represents a CDK program.
*/
export class App extends Root {
private prepared = false;

/**
* Initializes a CDK application.
* @param request Optional toolkit request (e.g. for tests)
Expand Down Expand Up @@ -58,7 +60,12 @@ export class App extends Root {
public synthesizeStack(stackName: string): cxapi.SynthesizedStack {
const stack = this.getStack(stackName);

this.node.prepareTree();
if (!this.prepared) {
// Maintain the existing contract that the tree will be prepared even if
// 'synthesizeStack' is called by itself. But only prepare the tree once.
this.node.prepareTree();
this.prepared = true;
}

// first, validate this stack and stop if there are errors.
const errors = stack.node.validateTree();
Expand Down Expand Up @@ -91,6 +98,9 @@ export class App extends Root {
* Synthesizes multiple stacks
*/
public synthesizeStacks(stackNames: string[]): cxapi.SynthesizedStack[] {
this.node.prepareTree();
this.prepared = true;

const ret: cxapi.SynthesizedStack[] = [];
for (const stackName of stackNames) {
ret.push(this.synthesizeStack(stackName));
Expand Down
15 changes: 8 additions & 7 deletions packages/@aws-cdk/cdk/lib/cloudformation/resource.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import cxapi = require('@aws-cdk/cx-api');
import { Construct, IConstruct } from '../core/construct';
import { TagManager } from '../core/tag-manager';
import { capitalizePropertyNames, ignoreEmpty } from '../core/util';
import { capitalizePropertyNames, ignoreEmpty, PostResolveToken } from '../core/util';
import { CfnReference } from './cfn-tokens';
import { Condition } from './condition';
import { CreationPolicy, DeletionPolicy, UpdatePolicy } from './resource-policy';
Expand Down Expand Up @@ -215,18 +215,19 @@ export class Resource extends Referenceable {

return {
Resources: {
[this.logicalId]: deepMerge({
// Post-Resolve operation since otherwise deepMerge is going to mix values into
// the Token objects returned by ignoreEmpty.
[this.logicalId]: new PostResolveToken({
Type: this.resourceType,
Properties: ignoreEmpty(this, properties),
// Return a sorted set of dependencies to be consistent across tests
DependsOn: ignoreEmpty(this, renderDependsOn(this.dependsOn)),
Properties: ignoreEmpty(properties),
DependsOn: ignoreEmpty(renderDependsOn(this.dependsOn)),
CreationPolicy: capitalizePropertyNames(this, this.options.creationPolicy),
UpdatePolicy: capitalizePropertyNames(this, this.options.updatePolicy),
UpdateReplacePolicy: capitalizePropertyNames(this, this.options.updateReplacePolicy),
DeletionPolicy: capitalizePropertyNames(this, this.options.deletionPolicy),
Metadata: ignoreEmpty(this, this.options.metadata),
Metadata: ignoreEmpty(this.options.metadata),
Condition: this.options.condition && this.options.condition.logicalId
}, this.rawOverrides)
}, props => deepMerge(props, this.rawOverrides))
}
};
} catch (e) {
Expand Down
29 changes: 17 additions & 12 deletions packages/@aws-cdk/cdk/lib/core/tokens/encoding.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import { unresolved } from "./unresolved";
* works even when different copies of the library are loaded.
*/
export class TokenMap {
private readonly tokenMap: {[key: string]: Token} = {};
private readonly tokenMap = new Map<string, Token>();

/**
* Generate a unique string for this Token, returning a key
Expand Down Expand Up @@ -44,14 +44,14 @@ export class TokenMap {
* Returns a `TokenString` for this string.
*/
public createStringTokenString(s: string) {
return new TokenString(s, BEGIN_STRING_TOKEN_MARKER, `[${VALID_KEY_CHARS}]+`, END_TOKEN_MARKER);
return new TokenString(s, QUOTED_BEGIN_STRING_TOKEN_MARKER, `[${VALID_KEY_CHARS}]+`, QUOTED_END_TOKEN_MARKER);
}

/**
* Returns a `TokenString` for this string.
*/
public createListTokenString(s: string) {
return new TokenString(s, BEGIN_LIST_TOKEN_MARKER, `[${VALID_KEY_CHARS}]+`, END_TOKEN_MARKER);
return new TokenString(s, QUOTED_BEGIN_LIST_TOKEN_MARKER, `[${VALID_KEY_CHARS}]+`, QUOTED_END_TOKEN_MARKER);
}

/**
Expand Down Expand Up @@ -86,25 +86,30 @@ export class TokenMap {
* Find a Token by key
*/
public lookupToken(key: string): Token {
if (!(key in this.tokenMap)) {
const token = this.tokenMap.get(key);
if (!token) {
throw new Error(`Unrecognized token key: ${key}`);
}

return this.tokenMap[key];
return token;
}

private register(token: Token, representationHint?: string): string {
const counter = Object.keys(this.tokenMap).length;
const counter = this.tokenMap.size;
const representation = (representationHint || `TOKEN`).replace(new RegExp(`[^${VALID_KEY_CHARS}]`, 'g'), '.');
const key = `${representation}.${counter}`;
this.tokenMap[key] = token;
this.tokenMap.set(key, token);
return key;
}
}

const BEGIN_STRING_TOKEN_MARKER = '${Token[';
const BEGIN_LIST_TOKEN_MARKER = '#{Token[';
const END_TOKEN_MARKER = ']}';

const QUOTED_BEGIN_STRING_TOKEN_MARKER = regexQuote(BEGIN_STRING_TOKEN_MARKER);
const QUOTED_BEGIN_LIST_TOKEN_MARKER = regexQuote(BEGIN_LIST_TOKEN_MARKER);
const QUOTED_END_TOKEN_MARKER = regexQuote(END_TOKEN_MARKER);

const VALID_KEY_CHARS = 'a-zA-Z0-9:._-';

/**
Expand Down Expand Up @@ -133,10 +138,10 @@ class TokenString {

constructor(
private readonly str: string,
private readonly beginMarker: string,
private readonly idPattern: string,
private readonly endMarker: string) {
this.pattern = `${regexQuote(this.beginMarker)}(${this.idPattern})${regexQuote(this.endMarker)}`;
quotedBeginMarker: string,
idPattern: string,
quotedEndMarker: string) {
this.pattern = `${quotedBeginMarker}(${idPattern})${quotedEndMarker}`;
}

/**
Expand Down
14 changes: 11 additions & 3 deletions packages/@aws-cdk/cdk/lib/core/tokens/resolve.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { IConstruct } from '../construct';
import { containsListToken, TOKEN_MAP } from "./encoding";
import { RESOLVE_OPTIONS } from "./options";
import { RESOLVE_METHOD, ResolveContext, Token } from "./token";
import { isResolvedValuePostProcessor, RESOLVE_METHOD, ResolveContext, Token } from "./token";
import { unresolved } from "./unresolved";

// This file should not be exported to consumers, resolving should happen through Construct.resolve()
Expand Down Expand Up @@ -83,8 +83,16 @@ export function resolve(obj: any, context: ResolveContext): any {
if (unresolved(obj)) {
const collect = RESOLVE_OPTIONS.collect;
if (collect) { collect(obj); }
const value = obj[RESOLVE_METHOD](context);
return resolve(value, context);

const resolved = obj[RESOLVE_METHOD](context);

let deepResolved = resolve(resolved, context);

if (isResolvedValuePostProcessor(obj)) {
deepResolved = obj.postProcess(deepResolved, context);
}

return deepResolved;
}

//
Expand Down
17 changes: 17 additions & 0 deletions packages/@aws-cdk/cdk/lib/core/tokens/token.ts
Original file line number Diff line number Diff line change
Expand Up @@ -130,4 +130,21 @@ export class Token {
export interface ResolveContext {
scope: IConstruct;
prefix: string[];
}

/**
* A Token that can post-process the complete resolved value, after resolve() has recursed over it
*/
export interface IResolvedValuePostProcessor {
/**
* Process the completely resolved value, after full recursion/resolution has happened
*/
postProcess(input: any, context: ResolveContext): any;
}

/**
* Whether the given object is an `IResolvedValuePostProcessor`
*/
export function isResolvedValuePostProcessor(x: any): x is IResolvedValuePostProcessor {
return x.postProcess !== undefined;
}
36 changes: 25 additions & 11 deletions packages/@aws-cdk/cdk/lib/core/util.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { IConstruct } from "./construct";
import { IResolvedValuePostProcessor, ResolveContext, Token } from "./tokens";

/**
* Given an object, converts all keys to PascalCase given they are currently in camel case.
Expand Down Expand Up @@ -30,21 +31,34 @@ export function capitalizePropertyNames(construct: IConstruct, obj: any): any {
/**
* Turns empty arrays/objects to undefined (after evaluating tokens).
*/
export function ignoreEmpty(construct: IConstruct, o: any): any {
o = construct.node.resolve(o); // first resolve tokens, in case they evaluate to 'undefined'.
export function ignoreEmpty(obj: any): any {
return new PostResolveToken(obj, o => {
// undefined/null
if (o == null) {
return o;
}

if (Array.isArray(o) && o.length === 0) {
return undefined;
}

if (typeof(o) === 'object' && Object.keys(o).length === 0) {
return undefined;
}

// undefined/null
if (o == null) {
return o;
}
});
}

if (Array.isArray(o) && o.length === 0) {
return undefined;
/**
* A Token that applies a function AFTER resolve resolution
*/
export class PostResolveToken extends Token implements IResolvedValuePostProcessor {
constructor(value: any, private readonly processor: (x: any) => any) {
super(value);
}

if (typeof(o) === 'object' && Object.keys(o).length === 0) {
return undefined;
public postProcess(o: any, _context: ResolveContext): any {
return this.processor(o);
}

return o;
}
2 changes: 1 addition & 1 deletion packages/@aws-cdk/cdk/test/core/test.tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -364,7 +364,7 @@ export = {
});

test.done();
}
},
}
};

Expand Down
24 changes: 12 additions & 12 deletions packages/@aws-cdk/cdk/test/core/test.util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,43 +32,43 @@ export = {

'[]'(test: Test) {
const c = new Root();
test.strictEqual(ignoreEmpty(c, []), undefined);
test.strictEqual(c.node.resolve(ignoreEmpty([])), undefined);
test.done();
},

'{}'(test: Test) {
const c = new Root();
test.strictEqual(ignoreEmpty(c, {}), undefined);
test.strictEqual(c.node.resolve(ignoreEmpty({})), undefined);
test.done();
},

'undefined/null'(test: Test) {
const c = new Root();
test.strictEqual(ignoreEmpty(c, undefined), undefined);
test.strictEqual(ignoreEmpty(c, null), null);
test.strictEqual(c.node.resolve(ignoreEmpty(undefined)), undefined);
test.strictEqual(c.node.resolve(ignoreEmpty(null)), null);
test.done();
},

'primitives'(test: Test) {
const c = new Root();
test.strictEqual(ignoreEmpty(c, 12), 12);
test.strictEqual(ignoreEmpty(c, "12"), "12");
test.strictEqual(c.node.resolve(ignoreEmpty(12)), 12);
test.strictEqual(c.node.resolve(ignoreEmpty("12")), "12");
test.done();
},

'non-empty arrays/objects'(test: Test) {
const c = new Root();
test.deepEqual(ignoreEmpty(c, [ 1, 2, 3, undefined ]), [ 1, 2, 3 ]); // undefined array values is cleaned up by "resolve"
test.deepEqual(ignoreEmpty(c, { o: 1, b: 2, j: 3 }), { o: 1, b: 2, j: 3 });
test.deepEqual(c.node.resolve(ignoreEmpty([ 1, 2, 3, undefined ])), [ 1, 2, 3 ]); // undefined array values is cleaned up by "resolve"
test.deepEqual(c.node.resolve(ignoreEmpty({ o: 1, b: 2, j: 3 })), { o: 1, b: 2, j: 3 });
test.done();
},

'resolve first'(test: Test) {
const c = new Root();
test.deepEqual(ignoreEmpty(c, { xoo: { resolve: () => 123 }}), { xoo: 123 });
test.strictEqual(ignoreEmpty(c, { xoo: { resolve: () => undefined }}), undefined);
test.deepEqual(ignoreEmpty(c, { xoo: { resolve: () => [ ] }}), { xoo: [] });
test.deepEqual(ignoreEmpty(c, { xoo: { resolve: () => [ undefined, undefined ] }}), { xoo: [] });
test.deepEqual(c.node.resolve(ignoreEmpty({ xoo: { resolve: () => 123 }})), { xoo: 123 });
test.strictEqual(c.node.resolve(ignoreEmpty({ xoo: { resolve: () => undefined }})), undefined);
test.deepEqual(c.node.resolve(ignoreEmpty({ xoo: { resolve: () => [ ] }})), { xoo: [] });
test.deepEqual(c.node.resolve(ignoreEmpty({ xoo: { resolve: () => [ undefined, undefined ] }})), { xoo: [] });
test.done();
}
}
Expand Down

0 comments on commit 77b516f

Please sign in to comment.