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

tests run 3x slower than Jest #229

Closed
6 tasks done
sachinraja opened this issue Dec 20, 2021 · 15 comments · Fixed by vitejs/vite#6309
Closed
6 tasks done

tests run 3x slower than Jest #229

sachinraja opened this issue Dec 20, 2021 · 15 comments · Fixed by vitejs/vite#6309

Comments

@sachinraja
Copy link
Contributor

sachinraja commented Dec 20, 2021

Describe the bug

Tests run 3x slower than Jest for me.

Reproduction

vitest: https://github.com/sachinraja/zod-to-ts/tree/vitest
jest: https://github.com/sachinraja/zod-to-ts/tree/main

System Info

System:
    OS: Linux 5.15 Pop!_OS 21.10
    CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
    Memory: 5.03 GB / 15.44 GB
    Container: Yes
    Shell: 5.8 - /usr/bin/zsh
  Binaries:
    Node: 16.11.1 - ~/.asdf/installs/nodejs/16.11.1/bin/node
    Yarn: 1.22.15 - ~/.asdf/installs/nodejs/16.11.1/.npm/bin/yarn
    npm: 8.0.0 - ~/.asdf/plugins/nodejs/shims/npm
  Browsers:
    Brave Browser: 96.1.33.106
    Firefox: 95.0
  npmPackages:
    vite: 2.7.4 => 2.7.4 
    vitest: 0.0.100 => 0.0.100

Used Package Manager

pnpm

Validations

@Demivan
Copy link
Member

Demivan commented Dec 28, 2021

Profiled this issue. Most of the time is spent in vite doTransform. The biggest bar it transforming typescript. It is a dependency of zod-to-ts.

image

@antfu Shouldn't default deps.external option prevent vitest from transforming stuff in node_modules?

@antfu
Copy link
Member

antfu commented Dec 29, 2021

Did you try if externalizing typescript would work? By default, it detects the packages' format to check if the package is a valid CJS or ESM package, and will only inline it when it's not vaild (e.g. ships ESM syntax with in a CJS package)

@Demivan
Copy link
Member

Demivan commented Dec 29, 2021

  optimizeDeps: {
    exclude: ['typescript']
  },
  ssr: {
    external: ['typescript']
  },
  test: {
    deps: {
      external: ['typescript']
    }
  }

All three options did not work. I would assume that ssr.external should work, but it doesn't.

@sachinraja
Copy link
Contributor Author

I did some logging in shouldExternalize

export async function shouldExternalize(id: string, config: Pick<ExecuteOptions, 'inline' | 'external'>) {

And it's externalized without having to set anything in the config.

@Demivan
Copy link
Member

Demivan commented Dec 29, 2021

@antfu I have finally found it!:

https://github.com/vitejs/vite/blob/5a111cedf31f579e3b8c8af5c4442d2e0cd5aa12/packages/vite/src/node/plugins/importAnalysis.ts#L566-L571

With this it takes ~4s, without these lines ~700ms.
So vite is doing transformRequest internally bypassing all vitest logic. I don't see any way to disable this.

@antfu
Copy link
Member

antfu commented Dec 29, 2021

Oh interesting and nice finding! I think we could introduce an option to disable it. /cc @patak-dev

@EvHaus
Copy link
Contributor

EvHaus commented Dec 29, 2021

If it helps, I've added vitest to my benchmarking repo here which has a collections of unit tests which run against jasmine, jest and vitest. In my benchmarks, I'm seeing vitest perform about 2x slower than Jest and nearly 4x slower than Jasmine.

@Demivan
Copy link
Member

Demivan commented Dec 30, 2021

@antfu Can I work on this?

@Demivan
Copy link
Member

Demivan commented Dec 30, 2021

Ok. I see it is already done 🙂

@Demivan
Copy link
Member

Demivan commented Dec 30, 2021

@sachinraja Tests are taking ~800ms now.
And with threads disabled ~650ms.

@antfu
Copy link
Member

antfu commented Dec 30, 2021

Vitest v0.0.100 (your branch)

  Time  3.39s (in thread 24ms, 14064.13%)

Vitest v0.0.123

  Time  662ms (in thread 33ms, 1989.25%)

@sachinraja
Copy link
Contributor Author

Time  1.19s (in thread 73ms, 1625.70%)

Seeing this on v0.0.123 so I'd say this is fixed now. Thanks for looking so deeply into this @Demivan and thanks for implementing the fix @antfu!

@EvHaus
Copy link
Contributor

EvHaus commented Dec 31, 2021

I re-ran my benchmarks with vitest@0.0.123 & vite@27.10 and found some strange results.

Here's Jest as our baseline (17.702s avg):

  Time (mean ± σ):     17.702 s ±  0.466 s    [User: 46.418 s, System: 8.754 s]
  Range (min … max):   17.155 s … 18.758 s    10 runs

Here's Vitest with default configuration settings (41.493s avg):

  Time (mean ± σ):     41.493 s ±  0.257 s    [User: 113.290 s, System: 24.967 s]
  Range (min … max):   41.061 s … 41.959 s    10 runs

Here's Vitest with threads: false (11.512s):

  Time (mean ± σ):     11.512 s ±  1.899 s    [User: 11.974 s, System: 1.394 s]
  Range (min … max):   10.135 s … 15.737 s    10 runs

Is it expected that multithreaded execution would be 4x slower? I'm on a Quad-Core Intel Core i5.

Overall, this is really amazing news! Vitest is faster than Jasmine which is considered fastest in all test runner benchmarks I've seen. This is a huge achievement!

@Demivan
Copy link
Member

Demivan commented Dec 31, 2021

I saw that threads are slower too. Alter checking it with nodejs profiler I can see that workers are idling a lot.
Will investigate where bottleneck is.

@Demivan
Copy link
Member

Demivan commented Jan 1, 2022

tinylibs/tinypool#17

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

Successfully merging a pull request may close this issue.

4 participants