Eliminate redundant work in type inference #15863
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
sandersn
approved these changes
May 15, 2017
mhegazy
reviewed
May 15, 2017
src/compiler/checker.ts
Outdated
| if (symbol) { | ||
| for (let i = 0; i < depth; i++) { | ||
| const t = stack[i]; | ||
| if (getSymbolForInference(t) === symbol) { |
There was a problem hiding this comment.
why not storing getSymbolForInference(target) on the stack directly instead of calling it on every entry when we check
mhegazy
approved these changes
May 15, 2017
|
will it be part of 2.3.3 ? |
no. this is TS 2.4 bound. |
|
I'm using Version 2.4.0-dev.20170519 and I've found that the project I shared in #15443 no longer has this issue, however I have a separate project that doesn't compile due to this memory issue. Has anyone else found the same? Will require some work to share a test case. |
|
Was the other project compiling on 2.3? |
|
@mhegazy Here is the other project that runs out of memory when compiling on 2.4.0-dev.20170519 and 2.3. https://github.com/OliverJAsh/ts-2.4-memory-issue/compare/2.4.0-dev.20170519 |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
With this PR we eliminate redundant work in type inference. Previously, we would explore instantiations of the same generic type up to five levels deep in type inference (which mirrored what we do in relationship checking to detect infinitely recursive types). We now explore just one level as no further inferences will be made beyond that anyway.
This PR doesn't include a regression test as the original issue only reproduces with the fp-ts library. However, I have manually verified that the out of memory issue is fixed and the compile time for the repro is less than 1s. Also, I have verified there are no changes in the real world code suites.
Fixes #15443.