Skip to content

[Transforms] Optimize frequent paths in visitEachChild.#8873

Merged
rbuckton merged 9 commits into
transformsfrom
transforms-visitEachChildPerf
May 31, 2016
Merged

[Transforms] Optimize frequent paths in visitEachChild.#8873
rbuckton merged 9 commits into
transformsfrom
transforms-visitEachChildPerf

Conversation

@rbuckton
Copy link
Copy Markdown
Contributor

This change addresses performance issues related to the general behavior of visitEachChild. The current visitEachChild function is highly generic, which make it difficult for hosts to optimize. This change adds individual, more optimized visitEachChildOf* functions for the most common nodes in our current test suite.

Also fixes some issues when compiling processDiagnosticMessages following "jake clean".

@rbuckton rbuckton added the Domain: API: Transforms Relates to the public transform API label May 27, 2016
@rbuckton
Copy link
Copy Markdown
Contributor Author

CC: @vladima, @yuit, @sandersn

@rbuckton
Copy link
Copy Markdown
Contributor Author

@vladima gave me a thumbs-up offline, but I'll wait till later tonight before merging in case anyone else has comments.

Comment thread src/compiler/factory.ts

export function createCall(expression: Expression, argumentsArray: Expression[], location?: TextRange) {
const node = <CallExpression>createNode(SyntaxKind.CallExpression, location);
export function createCall(expression: Expression, typeArguments: TypeNode[], argumentsArray: Expression[], location?: TextRange, flags?: NodeFlags) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why you need to pass in typeArguments parameter? In the change, it seems that all caller will pass undefined? also since we are in emitting phase, would we typearguments matter, won't we get rid of it anyway?

Copy link
Copy Markdown
Contributor Author

@rbuckton rbuckton May 31, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just for consistency. The updateCall function needs typeArguments (which will be undefined currently in all cases) to know it needs to create a new CallExpression when we remove typeArguments during transformation. It's not the responsibility of updateCall to remove it for you. Since you could pass in an actual value for typeArguments to updateCall, it is generally more clear to allow createCall to accept the value. This is to prevent future issues if we reuse this logic elsewhere in the compiler.

@yuit
Copy link
Copy Markdown
Contributor

yuit commented May 31, 2016

👍

@rbuckton rbuckton merged commit 1b7a67e into transforms May 31, 2016
@rbuckton rbuckton deleted the transforms-visitEachChildPerf branch May 31, 2016 20:21
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Domain: API: Transforms Relates to the public transform API

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants