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

Should we need to consider the compile speed in a product code with type definitions of rx v5? #1356

Closed
tetsuharuohzeki opened this issue Feb 18, 2016 · 9 comments

Comments

@tetsuharuohzeki
Copy link
Contributor

Notice

Summary

As an additional information, I tried to upgrade to [email protected] from [email protected] for the private code of my day job.

This is the comparison (sorry, I forgot diagnostic options...):

This upgrading needs some migration to my code thus this is not simply comparison. But this indicates that the type definitions of rxjs v5 is more complex than v4.

By this result, I feel we might to need to consider the compile speed in a product code with type definitions of rx v5. Of course, I strongly hope TypeScript team would fix this problem in the compiler.

How should we resolve this?

@tetsuharuohzeki tetsuharuohzeki changed the title May we need to consider the compile speed of type definitions? May we need to consider the compile speed in a product code with type definitions of rx v5. Feb 18, 2016
@tetsuharuohzeki tetsuharuohzeki changed the title May we need to consider the compile speed in a product code with type definitions of rx v5. May we need to consider the compile speed in a product code with type definitions of rx v5? Feb 18, 2016
@tetsuharuohzeki tetsuharuohzeki changed the title May we need to consider the compile speed in a product code with type definitions of rx v5? Should we need to consider the compile speed in a product code with type definitions of rx v5? Feb 18, 2016
@kwonoj
Copy link
Member

kwonoj commented Feb 18, 2016

Build time should be one of topics may need to track down further, up until now it was mainly focused on build time of library itself instead of any consumer perspective. longer than 2h of build time seems really serious, is there way to replicate it with some simple code snippet can be shared?

As an initial thought (rough guess though) I think this is somewhat related with type inference by references inside of codebases, not by complex type signatures. RxJS4's type definition already have complex signature more than what current RxJS5 does have. Still need further isolation to see what's causing this.

@kwonoj
Copy link
Member

kwonoj commented Feb 18, 2016

/cc @david-driscoll also to share, he might be interested as well.

@david-driscoll
Copy link
Member

@saneyuki do you have multiple different project references to RxJS (nested modules, etc?). I noticed a similar issue (see microsoft/TypeScript#5695) this may be the problem you're seeing, but I'm not entirely certain. I haven't gotten back to it since my last comment there, but it sounds there is a possible workaround (less than ideal though)

@tetsuharuohzeki
Copy link
Contributor Author

@kwonoj

is there way to replicate it with some simple code snippet can be shared?

Ugh, I still have not found the simple test case for 2hr~ worst case....

By the way, in other my personal project (which is different from the above 2hr~ project), this commit improves the compile time dramatically about ~18x improvement. (see more details: karen-irc/karen#574 (comment))

As an initial thought (rough guess though) I think this is somewhat related with type inference by references inside of codebases, not by complex type signatures. RxJS4's type definition already have complex signature more than what current RxJS5 does have. Still need further isolation to see what's causing this.

I agree. We need to continue to tracking this.


@david-driscoll

do you have multiple different project references to RxJS (nested modules, etc?).

No. I just working to upgrade to v5 from v4, then I switched to v5 completely.

@gytisgreitai
Copy link

I've just tried to introduce RxJS v5 to my small nodejs project
And compile time increase is impressive with just couple of lines of RxJS code (bindNodeCallback, throw, of, flatMap in a single class). :)

#Typescript Version 1.9.0-dev.20160315 without @reactivex/rxjs
tsc --p tsconfig.json --outDir ../generated/ -d --diagnostics
Files:             107
Lines:           44517
Nodes:          161511
Identifiers:     61623
Symbols:       3838799
Types:           11879
Memory used:    92948K
I/O read:        0.01s
I/O write:       0.03s
Parse time:      0.60s
Bind time:       0.46s
Check time:      1.51s
Emit time:       0.28s
Total time:      2.85s

#Typescript Version 1.9.0-dev.20160315 with @reactivex/rxjs
tsc --p tsconfig.json --outDir ../generated/ -d --diagnostics
Files:             165
Lines:           45897
Nodes:          176476
Identifiers:     67587
Symbols:       7426360
Types:          661767
Memory used:   938877K
I/O read:        0.07s
I/O write:       0.08s
Parse time:      0.58s
Bind time:       0.38s
Check time:     14.43s
Emit time:       1.03s
Total time:     16.42s

#Typescript Version 1.8.7 without @reactivex/rxjs
Files:             107
Lines:           44426
Nodes:          160944
Identifiers:     61447
Symbols:       3827377
Types:           11844
Memory used:    93924K
I/O read:        0.07s
I/O write:       0.04s
Parse time:      0.64s
Bind time:       0.47s
Check time:      1.61s
Emit time:       0.29s
Total time:      3.01s

#Typescript Version 1.8.7 with @reactivex/rxjs
Files:             165
Lines:           45806
Nodes:          175909
Identifiers:     67411
Symbols:       7388831
Types:          649879
Memory used:   933061K
I/O read:        0.07s
I/O write:       0.08s
Parse time:      0.75s
Bind time:       0.52s
Check time:     17.63s
Emit time:       0.90s
Total time:     19.80s

MacBook Pro (Retina, 13-inch, Late 2013), 2,4 GHz Intel Core i5, 8 GB

@benlesh
Copy link
Member

benlesh commented Mar 15, 2016

I've been talking to the TypeScript folks, and we have a few fixes that will drastically improve this.

@kwonoj
Copy link
Member

kwonoj commented Mar 20, 2016

I assume latest PR #1475 is addressing this issue. @saneyuki , would you able to confirm if latest changes helps? Still there may be continuous improvement in further, I think immediate improvement will help to solve worst case scenarios.

@tetsuharuohzeki
Copy link
Contributor Author

I confirmed it's a time that we can close this issue. Thank you @vladima and all!

@lock
Copy link

lock bot commented Jun 7, 2018

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Jun 7, 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

No branches or pull requests

5 participants