Skip to content

Instantiate anonymous types less#7138

Closed
sandersn wants to merge 8 commits into
masterfrom
instantiate-anonymous-types-less
Closed

Instantiate anonymous types less#7138
sandersn wants to merge 8 commits into
masterfrom
instantiate-anonymous-types-less

Conversation

@sandersn

Copy link
Copy Markdown
Member

Fixes issue: #7097

This operates during instantiateType to skip instantiation of anonymous types whose declarations are not bound by the type mapper that will do the instantiation.

I'm not sure this is the right approach, but I want to get suggestions on the right way to do it so I thought a strawman would help the discussion.

This operates during `instantiateType` to skip instantiation of anonymous
types whose declarations are *not* bound by the type mapper that will do the
instantation.

Unfortunately, it fails for this types and union types, perhaps because
these do not have symbols. This types also don't have explicit type
parameters, so that may be the reason.

For example, for-of30 currently fails.
Each of the methods has a `typeof Class` argument which currently causes
an out-of-memory error when compiling a lot of these methods.
Since they have no static side.
Including aliases of intersection types.
Comment thread src/compiler/checker.ts Outdated
function createUnaryTypeMapper(source: Type, target: Type): TypeMapper {
return t => t === source ? target : t;
const mapper = <TypeMapper>(t => t === source ? target : t);
mapper.mapsType = sym => sym.name === source.symbol.name;

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.

Is this like some sort of optimization?

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.

This is the information that mappers previously did not expose -- which types they map. This is important because hasTypeParametersInScope will now tell its caller whether a type needs to be instantiated by a particular mapper.

This is only partly an optimization, because some of the types for which we now skip instantiation should never be instantiated, like the anonymous typeof ListWrapper types in the generic static functions in the test.

1. Use types directly instead of type's symbol's names. Comparing names was
incorrect and bluebird's .d.ts from DefinitelyTyped still hit the bug.
2. Use this-type of classes directly to see whether this-types are mapped.
This is more complicated than the angular example in that it has a lot of
generic statics inside a class that is itself generic. The class and
functions all use the same name for their type parameters.
@sandersn

sandersn commented Mar 7, 2016

Copy link
Copy Markdown
Member Author

Weird. The linter failed on protocol.d.t.s, which this PR doesn't touch. But only on the push CI build, not the PR one.

@DanielRosenwasser

Copy link
Copy Markdown
Member

It's possible you'll need to merge in from master to sync up with some unrelated changes.

@DanielRosenwasser

Copy link
Copy Markdown
Member

Are there any specific speed changes you see when compiling tsc with the new version?

@sandersn

sandersn commented Mar 8, 2016

Copy link
Copy Markdown
Member Author

I haven't looked, except to test on all the RWC projects -- angular2 and bluebird.d.ts both ran out of memory before and now run just fine. But there should be plenty of unneeded instantiations that are no longer happening. I'll try to run some perf tests later.

@ahejlsberg

Copy link
Copy Markdown
Member

@sandersn Just merged #7448 so I think we can close this one.

@mhegazy mhegazy closed this Mar 9, 2016
@mhegazy mhegazy deleted the instantiate-anonymous-types-less branch March 9, 2016 20:45
@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.

5 participants