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

Expand Perf Suite Codebases, Expand Perf Suite Operations #44033

Closed
DanielRosenwasser opened this issue May 10, 2021 · 23 comments
Closed

Expand Perf Suite Codebases, Expand Perf Suite Operations #44033

DanielRosenwasser opened this issue May 10, 2021 · 23 comments
Assignees
Labels
Domain: Performance Reports of unusually slow behavior Infrastructure Issue relates to TypeScript team infrastructure Rescheduled This issue was previously scheduled to an earlier milestone

Comments

@DanielRosenwasser
Copy link
Member

The perf suite code typically was added for specific scenarios, but if you're trying to get a sense of how sourcemap emit has changed, it's hard given that not all projects use that functionality.

I think we should

  • Have perf suites projects enable emit
  • Have perf suites projects enable source map emit
  • Have perf suites projects enable declaration emit
@DanielRosenwasser DanielRosenwasser added Domain: Performance Reports of unusually slow behavior Infrastructure Issue relates to TypeScript team infrastructure labels May 10, 2021
@DanielRosenwasser DanielRosenwasser added this to the TypeScript 4.4.0 milestone May 10, 2021
@weswigham
Copy link
Member

@rbuckton we can turn on declaration emit, source maps, and declaration maps for most (if not all) perf projects (and remove noEmitOnError), but doing so is going to invalidate historic relationships for the data, so if we want to, we'll have to regenerate some historical data - are we OK with doing that?

@MartinJohns
Copy link
Contributor

@weswigham Perhaps you can do this additionally?

@DanielRosenwasser
Copy link
Member Author

(CC @amcasey)

@MartinJohns can you elaborate?

@MartinJohns
Copy link
Contributor

@DanielRosenwasser Continue to run the perf suite as is, but additionally run the perf suite with declaration emit, source maps and declaration maps enabled. Basically produce two data sets.

@rbuckton
Copy link
Member

In our current setup, we run benchmarks against a snapshot of a specific project, using the settings specific to that project. This is intended to cover different scenarios, such as:

  • Single-file output vs Multi-file output vs no output.
  • No source maps, vs external source maps, vs internal source maps.
  • Declaration emit vs. no declaration emit.
  • Emitting with and without comments.
  • Exercising different parts of the compiler (i.e., Compiler-Unions heavily uses union types to stress test those scenarios).

That way we aren't only focused on everything being a "kitchen sink" build, and we can catch performance regressions in non-"kitchen sink" areas (i.e., we don't want to slow down in code paths where we don't emit source maps). The current approach is somewhat randomized, but it covers most of the bases. I wouldn't want to have to run a benchmark against the same project for every permutation of every feature, as that doesn't scale.

@DanielRosenwasser Continue to run the perf suite as is, but additionally run the perf suite with declaration emit, source maps and declaration maps enabled. Basically produce two data sets.

What we can do is add new scenarios that point to the same sources but with different settings. That way old runs wouldn't be invalidated. So in that case we might have a "TFS+SourceMaps" scenario, etc.

@amcasey
Copy link
Member

amcasey commented Aug 2, 2021

Related: #44251

@jakebailey
Copy link
Member

jakebailey commented Mar 22, 2023

Oh, I didn't realize we were listing things here. Copy/paste from our internal thread and my notes:

  • mongoose
  • styled-components
  • @emotion/*
  • fp-ts
  • aws-sdk
  • bunp
  • artificial auto-imports benchmark (tsserver)
  • prisma
  • zod
  • Code like in Performance regression from #48044 #52345
  • vscode
  • compiler (updated to current year)
  • some repo with lots of ts-ignores
  • vuelidate
  • something with incremental parsing

@jakebailey
Copy link
Member

Also related, but, I want to add two new hosts to ts-perf, vscode and bun.

The former we can pull via https://www.npmjs.com/package/@vscode/test-electron and then use ELECTRON_RUN_AS_NODE. I want this because electron's build of v8 has different options, mainly pointer compression, which impact the maximum memory usage of the process as well as the small integer optimization.

I also bun's new "as node" mode working with some tweaks to typescript (which are really just bugs in bun that don't have proper error messages). It's an interesting test as it's not v8, it's JSC.

@DanielRosenwasser
Copy link
Member Author

Maybe Ghost.

@DanielRosenwasser DanielRosenwasser changed the title Turn on more thorough functionality on all perf suites Expand Perf Suite Codebases, Expand Perf Suite Operations Mar 22, 2023
@rubiesonthesky
Copy link

I have understood that the snapshots for benchmarks can be quite old, for example, Angular. Would it make sense in some point to refresh those? I would be interested how recent Angular code base is handled with different Typescript releases. I understand that you probably do not want to update the benchmarks you are doing in PRs. But maybe when doing release, it would be possible to add more current code. (And if you choose now one version, of course it will be old in few years :D ) Newer snapshots would be using new features.

Of course, it's also possible that I have misunderstood the case.

@jakebailey
Copy link
Member

Our hope is that we have this all in public, so things can be updated where needed by anyone. That and time / fragility are the reasons we haven't updated.

The snapshotting is hard to get rid of, because that's required for consistency in graphs without backfilling. However, I'm working on new infra that will be doing A/B tests, which means that we don't always need to be running a specific version if needed.

@DanielRosenwasser
Copy link
Member Author

Drizzle? (see #54939)

@jakebailey
Copy link
Member

As of today, our benchmark suite can now run bun. For example: #52656 (comment)

This is particularly interesting as it uses an entirely different engine (JSC) than Node/Deno (V8).

bun 0.8.1 is currently broken when running tsc (oven-sh/bun#4327), but one can run 0.7.3 and it works. tsserver tests don't work, nor do tsserver startup tests, but given I'm using an old release, hopefully it is fixed in a future release.

I'm not yet sure if this should be a part of our main test suite; probably not until we increase our capacity a bit more.

Next up would be using VS Code's electron build via ELECTRON_RUN_AS_NODE, which has different performance behaviors due to pointer compression and memory caging; we don't currently test it, yet there's probably more CPU cycles spent in tsserver via VS Code than any other way to run TypeScript!

@Jarred-Sumner
Copy link

bun 0.8.1 is currently broken when running tsc (oven-sh/bun#4327), but one can run 0.7.3 and it works. tsserver tests don't work, nor do tsserver startup tests, but given I'm using an old release, hopefully it is fixed in a future release.

This is fixed in oven-sh/bun#4378 (currently the canary build) which will become the release build (v0.8.2) tomorrow or the next day. The error was in some code assuming /dev/fd/${n} is mounted which it is not mounted by default in minimal Linux distributions like alpine. Our CI machine uses Ubuntu Server which mounts /dev/ by default, which is why we didn't catch it sooner. We will add an integration test that runs tsc --help.

The last few lines of output of strace tsc --help when tsc is run in bun:

Bun v0.8.1 Bun v0.8.2
image image

Note how it is no longer calling openat() on /dev/fd/1 in v0.8.2

@jakebailey
Copy link
Member

Thanks for the fix! There are other things that are broken (as noted above), but I have yet to go reproduce them. I'll send some bug reports once I get that sorted.

@Jolg42
Copy link

Jolg42 commented Aug 30, 2023

Hello from Prisma 👋🏼
Let us know if you're interested to hear from us about some complex types to test. It could be interesting to have some types used in Prisma Client Extensions, for example.

The performance test suite is located at https://github.com/microsoft/typescript-benchmarking? but is currently private? Do you plan to make it public one day? (Of course you might have reasons to not make it public, I'm curious here)

@jakebailey
Copy link
Member

That repo is currently only build infra, which I plan to make public in short order as there's nothing sensitive.

The actual tool that does the benchmarking isn't ready to be made public and that's where the benchmarks currently live. I'm hoping I can fix it up to pull benchmarks from the public repo but it's not really made for multiple places to store benchmarks, or benchmarks that have to be cloned from elsewhere. But that's just details and I'm working on it. The new hosts were just a lot easier to add!

@jakebailey
Copy link
Member

perf test vscode is now live: #55267 (comment)

Next up is adding some more benchmarks and making public what I can make public quickly, e.g. the infra itself (but not the tool, sorry).

@jakebailey
Copy link
Member

I'm happy to say that the new perf infra is now live and public: https://github.com/microsoft/typescript-benchmarking

I've been waiting to make this public as I did not yet have a good mechanism to support public benchmarks, but I got it all working in the past few days and now it's ready to go. The first public benchmarks are vscode, as well as self tests of TypeScript compiling itself (both tsc -p ./src/compile and tsc -b ./src).

Note that the public benchmarks are still split off from our old internal benchmarks; we have a lot more perf capacity than we used to have but still not enough to start adding lots that always run.

For now, they can be run via the perf test public preset. An example: #56987 (comment)

This is the first time we've had new, actually error-free benchmarks in years, so I'm very excited.

@jakebailey
Copy link
Member

I'm going to close this issue in favor of microsoft/typescript-benchmarking#32 on the benchmarking repo.

This thread was refocused toward benchmark suggestions, but now that the repo that holds them is public, discussion and PRing can happen there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Performance Reports of unusually slow behavior Infrastructure Issue relates to TypeScript team infrastructure Rescheduled This issue was previously scheduled to an earlier milestone
Projects
None yet
Development

No branches or pull requests