TypeScript: Checking is slow in project consuming xstate types

The original project is here: https://github.com/AlCalzone/node-zwave-js The actual owner is @AlCalzone

  1. Clone https://github.com/amcasey/ZwavePerfRepro
  2. npm ci
  3. tsc -p .

The portion of the code below // ACASEY boils down to a single function call that takes the vast majority of the checking time. The portion of the code above is enough scaffolding to make it compile.

I’ve framed the problem in terms of tsc because it’s easier to investigate, but the editor experience is also terrible.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Comments: 46 (39 by maintainers)

Most upvoted comments

probably want "strictFunctionTypes": true, because there are so many variance checks when using xstate types

I think I’ll pass 😨 (although the speedup is immense)

grafik

Aside from that, this breaks a bunch of typed function declarations (or at least makes them much less elegant).

In the original (i.e. unreduced) repro, I’m seeing

Before

Types:                       59271
Instantiations:             218383
Check time:                  4.41s

After

Types:                       42758
Instantiations:             149793
Check time:                  3.50s

I’ve prepared a simplified playground showcasing why this matters - look at the declared config and the cond property in it.

The current version (working): https://www.typescriptlang.org/play?#code/C4TwDgpgBAyglgOwOYBsIHkBOBBTmCGIAPACoB8UAvFCQNoC6UAPjQNwBQ7iwEmAZvgDG0AKIA3CAmDoARgCsIg4FADe7KFFCQAXFADOwTIiQcAvpy3QSBBHrjA4Ae1sBhZ3zhIAsvjCk3UhAAHsAANDTiksrBPAgAJnpQkVKyCkoU1GoatADSUIgRElK0AESWJfT0APy61vi29k4IAR5IWCT4mEgQwETqGjQBPCGh-RokydEhkgmqmuAQunmmUFWFUVC6CBASmP1kZhzsljQ2dg7OLZ4dXT1U+obGzFAArvEQHttxR9y8AsKnernJpXJD+ZzDMLrKRQGIzRKTVKKYAULJQQTOOI1KAACiUQVqQxi4R2tUmAEoqBQZI5HGh6hwNMBOt1gLUzo1Lu5riyemYLAtAQ0Ls1uW1MDdWX1xkSRv0JkUprFZoj5Mj2BlYMY0FhcARiHVhSCxZK7ixDcCuQhWuDAiNoSiDgLIFBQbbIeEFRs4fEEYqkel7mjnNiLZzXGKfH4SLKoV6pE7TEcMbZlII+EhdG7+iooHF8MzdCUALaOOIQFAlKCmUYaXOWIsAMXQ6BKrCgYEwjjAWxexZkvGrz3rCyLACFsAAlNsdrs9ubbAwQOK6AxGZBD8ya4MIXRojQTyd7sYDZm3NlQEoyfAAL2M+BKtYGGhTK9xAH18SSxJTKKiT8+mA9C8mAILCYgAHSdt2EGLjwcRUJQ1AlECADuvBtgB1ZPhoNb9HhSbsEAA

Proposed version (not working): https://www.typescriptlang.org/play?#code/C4TwDgpgBAyglgOwOYBsIHkBOBBTmCGIAPACoB8UAvFCQNoC6UAPjQNwBQ7iwEmAZvgDG0AKIA3CAmDoARgCsIg4FADe7KFFCQAXFADOwTIiQcAvpy3QSBBHrjA4Ae1sBhZ3zhIAsvjCk3UhAAHsAANDTiksrBPAgAJnpQkVKyCkoU1GoatADSUIgRElK0AESWJfT0APy61vi29k4IAR5IWCT4mEgQwP7OPCHhJMnAZGYc7JY0NnYOzi2eHV09VPqGxsxQAK7xEB4IEHET3LwCwtP1s00LSH2Bg4VRUDGSCUlF0vKKo6rqUILOOI1KAACiUQVqAQGYWeYlqIwAlFQKDJHI40PUOBpgJ1usBajNGvN3ItcT0zBZwFZCXNmiS2pglniiH8SFCYqFWSNniFXokRqlvuwMrBjGgsLgCMQ6g1aTcmSsWDKrsSEK07tChiMyGNKZAoDcNRzHlIebE3gKvulVlkoM5gcqia56T4-Gz+sbhh9daYJgDbMpBHwkLpDX8VFA4vgcboSgBbRxxCAoEpQUycjQRyyxgBi6HQJVYUDAmEcYF0CC2cZkvDTmyzVNjACFsAAlQvF0vl1RQA4GQ66AxGZB18wi23OXS2jQt1tTv4abFk-FQEoyfAAL2M+BKGcXGn9cV0IIA+uDwhAxEjKBRp-uoJgeltMAhYQA6Etlt99nhxKiUagSkuAB3XhCwXRd0wgqCNCg312CAA

Im also wondering about regressions as new versions of XState are released. Would it be worth having some sort of “TS Performance Test Suite” that runs on the CI?

That would be awesome. New versions of XState will be largely compatible with the current version, and typings will be similar. @mattpocock and @Andarist are also working on a TypeGen which will assist greatly with typing machines without compromising performance.

@AlCalzone it should be about 50/50 completion and squiggles - they’re doing the same work (separate discussion 😐).

I think I see another hotspot and I’m investigating now.

Finding # 1: @AlCalzone, you (and other consumers of xstate) probably want "strictFunctionTypes": true, because there are so many variance checks when using xstate types. Forcing the variance checks will probably also speed things up (because they’re cheaper than structural checks).

@AlCalzone I only remember making xstate changes to speed up your project. Having said that, I’m not aware of important perf improvements that slipped from 4.0 so, if we previously discussed some, yes, they probably landed.

@Andarist sorry, my investigation notes tend to be pretty stream-of-consciousness. 😄

Not a problem, I definitely tend to stream my consciousness on others as well as my thoughts race 😅

My mental model (which probably isn’t precisely correct) is that (T1 | T2) extends TCond ? TTrue : TFalse is treated as (T1 extends TCond ? TTrue : TFalse) | (T2 extends TCond ? TTrue : TFalse)

Also not sure if precisely correct, but my mental model is exactly the same for this 😉

I would agree that it’s generally best to avoid intersection types as much as possible.

Interesting, generally I wouldn’t have guessed that this might affect things that much but clearly it might - given this issue.


As to the final proposed improvement - TransitionsConfigMap seems to be exactly the same as we currently have, apart from the { event?: never } part. Adding this { event?: never } is unfortunately not technically correct. TEvent can be parametrized with { type: 'event' } after all. If that would improve our perf dramatically we could add this not-so-correct change, but if we could avoid it - then this would be preferred.

Looking at the proposed TransitionsConfigArray: so this trick uses conditional types distribution to handle this rather than mapped types. Nice trick. Gonna prepare a PR with this change in a moment as the current version is incorrect anyway from what I’m seeing.


Question - would you be open for chatting a little bit about TS performance and guidelines? We’d love to improve the situation in this regard but, quite frankly, there are nearly no resources about the subject. We are also working on a typegen tool that potentially could help with this, but it would also be very helpful for us if we would know what patterns should we stick to there, what patterns should we avoid, etc.

@Andarist sorry, my investigation notes tend to be pretty stream-of-consciousness. 😄 I tried to indicate the final state in my “to summarize” comment and the numbers reflect the performance impact of those changes.

Having said that, I’d hold off a bit longer because we can’t actually figure out why that change helps so much (or just drop { event?: never } - the rest of the change seems sound).

Regarding your current code, it took me a while to figure that out too, because I always forget that conditionals distribute over unions. My mental model (which probably isn’t precisely correct) is that (T1 | T2) extends TCond ? TTrue : TFalse is treated as (T1 extends TCond ? TTrue : TFalse) | (T2 extends TCond ? TTrue : TFalse).

I did try TEvent & { type: K } since that seemed most natural to me too, but it caused a huge slowdown. I’m planning to isolate that and file a fresh bug (against us, not you).

I would agree that it’s generally best to avoid intersection types as much as possible.

Purely for your amusement, I thought this might help, but it makes checking 4X as slow. 😆

TransitionConfigOrTarget<TContext, TEvent & { type: K }>

Thanks, @Andarist, examples like that are very helpful! I always forget that conditionals distribute across unions. If that’s what you’re going for, IMHO (as someone who totally failed to understand this, so grain of salt…) is that it would be more straightforward to have the conditional outside the generic type: TEvent extends { type: K } ? TransitionConfigOrTarget<TContext, TEvent> : never.

I’ll ask around in case there’s a more direct way to express that.

Edit: no, what you have seems good. And 10% seems like a fair price to pay for that kind of inference improvement.

Edit 2: I haven’t dug into it, but my version doesn’t actually work. Stick with what you’ve got. 😄

Unfortunately I dont currently have a spare minute to help with this issue but I just want to throw in my words of encouragement.

This is really important work for all us TS users and im glad you guys @amcasey and @davidkpiano are working on it. It sadly sounds like theres going to be some tradeoffs of performance vs type safety here.

Im also wondering about regressions as new versions of XState are released. Would it be worth having some sort of “TS Performance Test Suite” that runs on the CI?

@amcasey It’s not intentional; it should.

Nice, that’s quite the improvement. Please let me know if there are other optimizations I can make in XState and I will prioritize those.

@davidkpiano @amcasey Just did a quick test using today’s TypeScript and xstate from https://github.com/davidkpiano/xstate/issues/1350#issuecomment-668852857. The original repro I’ve sent @amcasey via mail now takes 8s on my NAS instead of over 30. On my desktop, we’re talking 1-2 seconds instead of 5. Although I’m not sure how much of that is the request for completions.

Thanks everyone, I will test it out after my vacation 👍🏻