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

[Feature Request] Reduce TS parsing/evaluation times #1481

Open
milesj opened this issue Jul 30, 2024 · 1 comment
Open

[Feature Request] Reduce TS parsing/evaluation times #1481

milesj opened this issue Jul 30, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@milesj
Copy link

milesj commented Jul 30, 2024

Is your feature request related to a problem? Please describe.

We're profiling our TS monorepo and the @temporalio packages have popped up as a hot path. I've noticed a few issues that can probably be optimized:

  1. Importing the @temporalio/client package takes 1 second. In the flame chart below, you'll notice that our temporalclient.ts file, which imports @temporalio/client takes most of the compilation time.
Screenshot 2024-07-30 at 3 05 25 PM

This can maybe be optimized by removing barrel imports (giant index exports), and moving things into deep path imports. Additionally, this may be caused by #2.

  1. The @temporario/proto TS declaration file is 49k lines long and is 2.67 MB in size. The generated file at @temporalio/proto/protos/root.d.ts is massive and is most likely consuming most of the parsing/checking time.

This can maybe be optimized by splitting root.d.ts into multiple files so they can be checked in parallel. Probably split into coresdk.d.ts, temporal.d.ts, google.d.ts, grpc.d.ts, etc.

Describe the solution you'd like

Improved TS compiler speeds. I listed some possible solutions above.

Additional context

This is using project references and was tested simply using tsc --build.

@milesj milesj added the enhancement New feature or request label Jul 30, 2024
@mjameswh
Copy link
Contributor

mjameswh commented Aug 5, 2024

Importing the @temporalio/client package takes 1 second. In the flame chart below, you'll notice that our temporalclient.ts file, which imports @temporalio/client takes most of the compilation time.

This is not how I read your flame chart. Locating, reading and parsing source files to memory appears to be taking ~250ms. That's for all sources files used by that TS project, including loading the @temporalio/client and @types/node modules, and many more. You see these in your flame chart as the pink findSourceFile, green createSourceFile bars (and more).

The checkSourceFile step, which is taking 1.1 seconds in your flame graph, is for type checking of a single source file (i.e. your temporalclient.ts file); checkSourceFile is not recursive. That's indeed huge, especially given that this time corresponds to essentially two expressions.

If you look the details on these checkExpression, you should see the exact location of those expressions in your source file. What exactly are you doing there? My intuition is that you might be passing Client or Connection objects across subpackages of your mono repo, which are compiled against different copies of @temporalio/* packages. That forces TSC to assert that both type definitions are compatible, which can indeed be very long on complex types.

Also make sure you are using a recent version of TSC. They have been working hard recently on build-time performance.

The @temporario/proto TS declaration file is 49k lines long and is 2.67 MB in size. The generated file at @temporalio/proto/protos/root.d.ts is massive and is most likely consuming most of the parsing/checking time.

This can maybe be optimized by splitting root.d.ts into multiple files so they can be checked in parallel. Probably split into coresdk.d.ts, temporal.d.ts, google.d.ts, grpc.d.ts, etc.

On my local machine, findSourceFile on @temporalio/proto/protos/root.d.ts takes 124ms. That's indeed larger than most other modules, but not unusually large. For comparision, findSourceFile on @types/node/index.d.ts gives me 94ms. And unless skipLibCheck is sets to false in tsconfig.json (it is generally recommended to leave it to its default value, which is true), those large imports should not incur any time for type checking.

Splitting root.d.ts to multiple subfiles would therefore not make a significant difference (no more than a few tens of ms). TSC can't do parallel parsing/checking, and those namespaces depends on each others, so you'd end up loading most of them anyway. The only gain we may get would be from skipping the coresdk and temporalio.test namespaces, which are not used in the context of @temporalio/client, but these are rather small packages, and that gain might be offset by having to open and load more files.

If you still think this is an issue, would you mind providing more details on exactly how you measured this? And if possible, please include a sample repo so we know that we are measuring the same thing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants