Skip to content

Skip unnecessary instatiation of anonymous types#7448

Merged
ahejlsberg merged 3 commits into
masterfrom
optimizeInstantiation
Mar 9, 2016
Merged

Skip unnecessary instatiation of anonymous types#7448
ahejlsberg merged 3 commits into
masterfrom
optimizeInstantiation

Conversation

@ahejlsberg
Copy link
Copy Markdown
Member

Fixes #7097.

This PR is basically a revised implementation of #7138. It corrects #7138 by only optimizing in cases where the anonymous type to be instantiated isn't itself an instantiation. Also, the implementation of type mappers and type erasers is reworked to share more code.

@sandersn Let's discuss how to merge this with #7138. Perhaps we just pull the tests over to this PR.

Comment thread src/compiler/checker.ts
const declaration = <DeclarationWithTypeParameters>node;
if (declaration.typeParameters) {
for (const d of declaration.typeParameters) {
if (contains(mappedTypes, getDeclaredTypeOfTypeParameter(d.symbol))) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

why does it work to say d.symbol directly when normally you have to say getSymbolOfNode(node)? (for example, in the thisType case below).

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We know that type parameters are never subject to global symbol merging, so getSymbolOfNode(d) will always be the same as d.symbol. We do the same thing in a few other places in the compiler.

@sandersn
Copy link
Copy Markdown
Member

sandersn commented Mar 9, 2016

The fix looks good to me.

If you can think of a way to show that the fix really skips unneeded instantiations without needing a giant test to trigger out-of-memory, by all means do. (Maybe the symbols file? I don't much about its information.) Otherwise just grabbing the altered bluebird d.ts from my PR will work.

@ahejlsberg
Copy link
Copy Markdown
Member Author

Latest commit pulls over tests from #7138.

ahejlsberg added a commit that referenced this pull request Mar 9, 2016
Skip unnecessary instatiation of anonymous types
@ahejlsberg ahejlsberg merged commit 614afb7 into master Mar 9, 2016
@ahejlsberg ahejlsberg deleted the optimizeInstantiation branch March 9, 2016 20:40
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants