Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Design Meeting Notes, 5/10/2019 #31350

Closed
DanielRosenwasser opened this issue May 10, 2019 · 10 comments
Closed

Design Meeting Notes, 5/10/2019 #31350

DanielRosenwasser opened this issue May 10, 2019 · 10 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

Perf edition!

Fix relations of instantiations of the same signature

#31029

  • Problem is that when we relate signatures to each other, we erase type parameters to anys.
  • We have a fix for that, but nowadays whenever we have a fix we also have a break.
    • It blows up types for styled-components/v3 blows up with an out-of-memory (OOM) error.
    • And it breaks Ember's Observer type (which is actually invarian) and the addObserver method whose methods take a key and a value for the current type.
    • The more recent styled-components increases in type-checking time.
      • About 2 seconds.

Now we're talking about

Slow completions

  • Completions have slowed down nowadays as well, partially because we check against the this type to decide whether we should even show methods in the first place.
  • Wesley: The problem is that we do a fully structural check, but we fundamentally need to make that cheap somehow.
    • One idea is that we need to be able to keep the constraint from the false branch of conditionals to short-circuit operations (by not eagerly evaluating conditionals)
    • Introducing negated types would be problematic for reasoning about, display purposes.
    • Worth trying at least.
  • Could also try to avoid instantiations of conditionals using infer types.
  • We also don't cache the results of what conditional inference does.
  • One other thing is we need to be able to help inform people whether their DefinitelyTyped changes are going to cause regressions so we can judge whether more accurate types are "worth it".

Action items:

Caching control flow results across invocations

#31003

  • Minor slowdowns and increased memory usage by adding the change, but fixes crazy-long cases with assignments on the LHS.
  • Why doesn't the test case cause an issue?
    • Element access assignments can't be cached, these can.
  • Resolution: merge for 3.5.

Array spread emit fixes

#8856
#31166

  • The way that we emit array spreads in ES5 is incorrect because we use concat and that omits missing elements.
    • In --downlevelIteration, this works right, but not otherwise.
  • We had an initial version that was slower, but still didn't affect the compiler much.
    • A community member suggested something that's faster, is almost as fast as the original.
  • Resolution: approved, but for 3.6
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label May 10, 2019
@falsandtru
Copy link
Contributor

@DanielRosenwasser Why you don't address #31148?

@MartinJohns
Copy link
Contributor

@falsandtru Because time is limited, and they did not get to discuss your favorite issue.

@falsandtru
Copy link
Contributor

@MartinJohns Looks like you don't have any ability or position to evaluate the problem.

@MartinJohns
Copy link
Contributor

That's a rude and silly thing to say, but even if it would be true it would be irrelevant. You asked why they did not address your issue in the design meeting notes, and I gave you an answer. I don't need to have any special abilities or a position in the TypeScript team to give you this answer. It's just common sense.

The TypeScript team consists of humans, and they have limited time. They prioritize the issues and then discuss the issues on top of their list. Your issue was not on top of the list and there was not enough time to discuss all issues.

@falsandtru
Copy link
Contributor

falsandtru commented May 11, 2019

FYI, blocking a voice of the person concerned is no common sense. It is a behavior of dictators. Thanks.

@zpdDG4gta8XKpMCd
Copy link

might be related #30696 (comment)

@DanielRosenwasser
Copy link
Member Author

@DanielRosenwasser Why you don't address #31148?

The most critical thing we are dealing with right now is performance issues that were introduced in TypeScript 3.4. Array spread behavior is one of our most upvoted issues and it has a clear solution.

@falsandtru
Copy link
Contributor

falsandtru commented May 14, 2019

I've no objection to the current items but #31148 is an important and deep-rooted problem which breaks TypeScript ecosystem on npm. So I think you should keep your team informed about it to find a solution.

@RyanCavanaugh
Copy link
Member

@falsandtru thanks for your feedback on prioritization. We can't change what we discussed in a meeting that occurred in the past, but will keep evaluating feedback in this area. You might also want to track #18588 since it relates to your concerns.

@falsandtru
Copy link
Contributor

falsandtru commented May 14, 2019

Thanks for your correct info. I move to there. However, I think that solution will be a workaround but probably it is not the best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Notes Notes from our design meetings
Projects
None yet
Development

No branches or pull requests

5 participants