Skip to content
This repository has been archived by the owner on Jan 22, 2025. It is now read-only.

[experimental] Speed up Typescript without reducing overloads in json-rpc-types #1682

Closed
steveluscher opened this issue Oct 5, 2023 · 6 comments · Fixed by #1760
Closed
Labels
bug Something isn't working released

Comments

@steveluscher
Copy link
Contributor

steveluscher commented Oct 5, 2023

Overview

The new RPC API uses function overloading liberally to describe the different outputs you can get from the RPC depending on the inputs you supply. You can inspect these utility types here.

Steps to reproduce

  1. Compile everything with pnpm compile
  2. Go into the library and profile the typechecker
    cd packages/library/
    pnpm tsc -p ./tsconfig.json --noEmit --generateTrace tracing_output_folder
    
  3. Look for hotspots
    pnpx @typescript/analyze-trace tracing_output_folder/
    
  4. Open the trace in chrome://tracing for more detail

Description of bug

Typechecking, and thereby autocomplete, has slowed precipitously as a result of recursion in the checker. If you cut these overloads in half, you get a ~30x speedup.

Without reducing the number of overloads (we need them for methods like getAccountInfo) speed up the typechecker by making the types more efficient.

Required reading: https://github.com/microsoft/TypeScript/wiki/Performance

@steveluscher steveluscher added the bug Something isn't working label Oct 5, 2023
@nmn
Copy link

nmn commented Oct 18, 2023

The biggest issue I see is that PendingRpcRequestBuilder and PendingRpcSubscriptionBuilder types are meant to just wrap the return type of functions.

The problem that you're trying to solve is that if a functions is of the type:

((...A1) => R1) & ((...A2) => R2)

You want the following result:

((...A1) => PendingRpcRequest<R1>) & ((...A2) => PendingRpcRequest<R2>)

and not

(...A1 | A2) => PendingRpcRequest<R1 | R2>


I'll need some help actually testing these changes, but I wonder if something like this would work:

type PendingRpcRequestBuilder<TOverloadedMethod> = 
  TOverloadedMethod extends { (...args: infer A): infer R } & infer Rest
    ? { (...args: A): PendingRpcRequest<R> } & PendingRpcRequestBuilder<Rest>
    : TOverloadedMethod;

type PendingRpcSubscriptionBuilder<TOverloadedMethod> =
  TOverloadedMethod extends { (...args: infer A): infer R } & infer Rest
    ? { (...args: A): PendingRpcSubscription<R> } & PendingRpcSubscriptionBuilder<Rest>
    : TOverloadedMethod;

These types should do what is intended, but that really depends on how TS infers the Rest in the expressions.

If this works, it removes the need for a bunch of indirection where:

  1. we convert method overloads to a tuple,
  2. then convert each type in the tuple,
  3. then create a union; and
  4. then convert the union back into a tuple!

I'm not sure how to test my changes so could you try it on your end?

@mcintyre94
Copy link
Contributor

mcintyre94 commented Oct 19, 2023

Hey @nmn thanks for the suggestion! Unfortunately it doesn't seem to be working

I tested it by updating those types in packages/rpc-transport/src/json-rpc-types.ts, and then running pnpm test:typecheck in packages/rpc-transport

I get errors showing that functions on rpc are no longer callable:

src/__tests__/json-rpc-test.ts:105:38 - error TS2349: This expression is not callable.
  Type '[(...args: unknown[]) => unknown, (...args: unknown[]) => unknown, (...args: unknown[]) => unknown, (...args: unknown[]) => unknown, (...args: unknown[]) => unknown, (...args: unknown[]) => unknown, (...args: unknown[]) => unknown, (...args: unknown[]) => unknown, (...args: unknown[]) => unknown, (...args: unknown[]...' has no call signatures.

105             const result = await rpc.someMethod().send();

And similar errors when I typecheck eg packages/rpc-core which uses the transport:

src/rpc-subscriptions/__typetests__/vote-notifications-type-test.ts:22:22 - error TS2349: This expression is not callable.
  Type '[(NO_CONFIG?: Record<string, never> | undefined) => Readonly<{ hash: Blockhash; signature: TransactionSignature; slots: readonly bigint[]; timestamp: UnixTimestamp | null; votePubkey: Base58EncodedAddress; }>]' has no call signatures.

22     rpcSubscriptions.voteNotifications().subscribe({ abortSignal: new AbortController().signal }) satisfies Promise<

@nmn
Copy link

nmn commented Oct 19, 2023

Thanks for the steps to test, I needed that.

Perhaps the "Rest" is not getting inferred correctly. I have one more idea that does less than this does. Let me see if that helps.

@steveluscher
Copy link
Contributor Author

I took a crack at this today from the perspective of caching rather than speed. I don't mind it being slow; what I mind is that it seems to do a full recheck on every keystroke, and I don't know why. Here's a realtime screencast of me typing a period, deleting it, then typing it again.

Screen.Recording.2023-10-20.at.10.04.48.PM.mov

That's >15s per keystroke to get Intellisense.

I'd be fine with that taking 15s once, and then being instant from then onward.

@github-actions
Copy link
Contributor

🎉 This issue has been resolved in version 1.87.3 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link
Contributor

github-actions bot commented Nov 3, 2023

Because there has been no activity on this issue for 7 days since it was closed, it has been automatically locked. Please open a new issue if it requires a follow up.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 3, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Something isn't working released
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants