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

Add Babel scenario #17

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

Conversation

liuxingbaoyu
Copy link

@liuxingbaoyu liuxingbaoyu commented Jan 11, 2024

Thank you very much for your work on the benchmark!

Babel seems to be a good scenario, a lot of typescript code, and widely used.

BTW: Babel has the incremental option enabled, I'm not sure if I need to do anything extra.

@liuxingbaoyu liuxingbaoyu changed the title Add Babel Add Babel scenario Jan 11, 2024
@jakebailey
Copy link
Member

Thanks for the PR; the repo is in flux and benchmarking resources are limited, so I'm not sure I'm going to take this quite yet.

Is there something special about Babel that warrants its addition besides it just being a popular library? All of the current benchmarks are "special" in that they test codebases that are slow or often become slow for one reason or another. It's likely that the list from microsoft/TypeScript#44033 will come first.

@liuxingbaoyu
Copy link
Author

Because there are all JS/TS/FLOW AST type definitions in @babel/types and @babel/traverse, and we support the form NodePath.get("body[0].expression"), It's very slow.
And there have been times in the past where their behavior became extremely slow either in the Babel repo or downstream.
microsoft/TypeScript#50290 (comment)
microsoft/TypeScript#50515

So personally it seems to me that Babel might be a suitable typical example to measure this kind of usage. :)
By the way, at least last year we tested tsc would OOM with 6GB when strictNullChecks was enabled in Babel. :)

Of course, if you think Babel is not suitable for adding to this repository, I completely understand. The project itself is amazing, and it’s great to see it coming!

@jakebailey
Copy link
Member

I've done a bunch of refactoring and so this PR is no longer up to date with how to add a benchmark; a better example is #7, if you wanted to update this PR.

@liuxingbaoyu
Copy link
Author

I didn't add it to scripts/src/generateMatrix.ts because it seemed like it would require some knowledge of CI.

@jakebailey
Copy link
Member

It shouldn't, other than picking an agent for the baseline to run on long term, and knowing roughly how long it takes to do a single compile (which I could measure for you).

Copy link
Member

@jakebailey jakebailey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Until we figure out how to safely install the repo, I unfortunately cannot merge this.

Additionally, the scripts need to be chmod -x'd.

cases/scenarios/babel/setup.sh Outdated Show resolved Hide resolved
@jakebailey jakebailey self-requested a review April 15, 2024 22:15
@jakebailey
Copy link
Member

FWIW I'm waiting on this one until the babel repo's layout is finalized. There's also a concern that the benchmark takes too long if the new project references hurt so much, that and the fact that build mode means that building projects may cause things to "fail fast" and cause the benchmark to stop working if TS ever adds new errors to babel's TS code.

@liuxingbaoyu
Copy link
Author

The speed problem may be solved by skipLibCheck: false.
fail fast is indeed a problem, luckily Babel automatically generates tsconfig, the worst case scenario may be that we keep an old script.

cc @nicolo-ribaudo You may have better ideas!

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

Successfully merging this pull request may close these issues.

2 participants