Skip to content

Commit

Permalink
perf: Replacing "else if" with a "switch case" statement to improve T…
Browse files Browse the repository at this point in the history
…ypescript performance (#1142)

This is a follow-up (and solution) to #1135 and #1141

I generated a trace from the build process and using
[typescript-analyze-trace](https://github.com/microsoft/typescript-analyze-trace)
to analyze the hotspots.

It turns out that time is spent almost exclusively in this kind of code:

```
if (object.choice?.$case === "aNumber" && object.choice?.value !== undefined && object.choice?.value !== null) {
  message.choice = { $case: "aNumber", value: object.choice.value };
} else if (
  object.choice?.$case === "aString" && object.choice?.value !== undefined && object.choice?.value !== null
) {
  message.choice = { $case: "aString", value: object.choice.value };
} else if (
//...
```

The previous PR I opened adding "else if" does nothing regarding
Typescript perf.

With "else if", my test file trace looks like this:

```
npx analyze-trace traceDir
Hot Spots
├─ Check file /home/dan/projects/ts-proto/integration/large/messages.ts (31108ms)
│  ├─ Check deferred node from (line 6267, char 3) to (line 6539, char 4) (13123ms)
│  ├─ Check deferred node from (line 15342, char 3) to (line 15591, char 4) (7092ms)
│  ├─ Check deferred node from (line 10849, char 3) to (line 11059, char 4) (4460ms)
│  ├─ Check deferred node from (line 17320, char 3) to (line 17496, char 4) (2146ms)
│  ├─ Check deferred node from (line 3720, char 3) to (line 3832, char 4) (610ms)
│  └─ Check deferred node from (line 8791, char 3) to (line 8915, char 4) (516ms)
└─ Check file /home/dan/projects/ts-proto/node_modules/typescript/lib/lib.dom.d.ts (1109ms)
```

Now, with this commit, I'm replace `else if` with a `switch` statement.

The analysis of the build looks like this:

```
Hot Spots
├─ Check file /home/dan/projects/ts-proto/integration/large/messages.ts (3139ms)
│  └─ Check deferred node from (line 6278, char 3) to (line 6573, char 4) (525ms)
└─ Check file /home/dan/projects/ts-proto/node_modules/typescript/lib/lib.dom.d.ts (617ms)
```

The biggest `switch` statement takes 525ms to analyze (down from
13seconds with `else if`)

Implementation-wise, the hardest part is knowing when to close the
switch statement. I'm doing a test at the beginning of each loop and at
the very end of the function.
  • Loading branch information
moufmouf authored Nov 28, 2024
1 parent c1e7691 commit de1a616
Show file tree
Hide file tree
Showing 8 changed files with 302 additions and 196 deletions.
67 changes: 37 additions & 30 deletions integration/oneof-unions-snake/google/protobuf/struct.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

55 changes: 37 additions & 18 deletions integration/oneof-unions-value/google/protobuf/struct.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

92 changes: 56 additions & 36 deletions integration/oneof-unions-value/oneof.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

65 changes: 37 additions & 28 deletions integration/oneof-unions/google/protobuf/struct.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit de1a616

Please sign in to comment.