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

[Docs] Explain how to use sharding #4269

Closed
6 tasks done
7rulnik opened this issue Oct 8, 2023 · 8 comments · Fixed by #5736
Closed
6 tasks done

[Docs] Explain how to use sharding #4269

7rulnik opened this issue Oct 8, 2023 · 8 comments · Fixed by #5736

Comments

@7rulnik
Copy link
Contributor

7rulnik commented Oct 8, 2023

Describe the bug

First of all, I am not sure it's a bug, feel free to convert to discussion or close it.

So we experienced performance problems with Vitest after migration from Jest. I am aware of eperimentalVmThreads existence, but we faced it before it was introduced. So I decided to play a bit with VITEST_MIN_THREADS and VITEST_MAX_THREADS. For all results captured from CI, we are using Amazon EC2 instances. In this case machines with 32 CPUs.

Our vitest config:

import { createRequire } from 'node:module';
import { defineConfig } from 'vitest/config';
import tsconfigPaths from 'vite-tsconfig-paths';
import react from '@vitejs/plugin-react';
import loadCssModule from 'vite-plugin-load-css-module';

process.env.TZ = 'utc';
process.env.ENV = 'test';
process.env.IS_VITEST = 'true';
process.env.BABEL_ENV = 'test';
process.env.NODE_ENV = 'development';
process.env.DOTENV_CONFIG_PATH = './.env.dist';

const require = createRequire(import.meta.url);
const babelConfig = require('./babel.config');

export default defineConfig({
  test: {
    include: [
      '{client,server,scripts,test_utils,build}/**/*.{vitest,test,spec}.{ts,tsx,js}',
    ],
    globals: true,
    setupFiles: './test_utils/vitest.setup.ts',
    testTimeout: 10_000,
    teardownTimeout: 5_000,
    server: {
      deps: {
        inline: ['@aviasales/ui', '@aviasales/analytics'],
      },
    },
    deps: {
      // Для зависимостей с кривыми ESM или только-CJS дефолтными экспортами
      // В нашем случае это lodash/fp и @loadable/component
      interopDefault: true,
    },
    cache: {
      dir: '.cache/vitest',
    },
    useAtomics: true,
    poolMatchGlobs: [
      [
        '**/client/src/product/usercom/services/subscriptions/__tests__/subscriptions.relations.test.ts',
        'child_process',
      ],
      [
        '**/client/src/product/explore/widgets/nearby_airports/models/visibility.model.vitest.ts',
        'child_process',
      ],
      [
        '**/client/src/product/usercom/services/passengers/components/document_form/inputs/nationality/__tests__/nationality.model.test.ts',
        'child_process',
      ],
      [
        '**/client/src/product/assisted/services/feedback/__tests__/purchase_feedback.vitest.ts',
        'child_process',
      ],
      [
        '**/client/src/product/assisted/services/feedback/__tests__/troubles_feedback.vitest.ts',
        'child_process',
      ],
      [
        '**/client/src/product/hotels/services/hotels_map/models/__tests__/hotels_map.model.test.ts',
        'experimentalVmThreads',
      ],
      [
        '**/client/src/product/premium/widgets/profile/features/bank_account_withdrawal_form/model/model.vitest.ts',
        'experimentalVmThreads',
      ],
      [
        '**/client/src/product/hotels/services/hotels_search/__tests__/hotels_search.model.test.ts',
        'child_process',
      ],
      [
        '**/client/src/product/explore/widgets/prices/__tests__/prices.vitest.tsx',
        'child_process',
      ],
      [
        '**/client/src/product/hotels/services/hotel/models/__tests__/hotel_map.model.test.ts',
        'child_process',
      ],
      [
        '**/client/src/product/explore/services/helios_forms/helios_forms.vitest.ts',
        'child_process',
      ],
      [
        '**/client/src/product/assisted/services/orders/__tests__/orders.model.test.ts',
        'child_process',
      ]
    ],
  },
  resolve: {
    alias: [
      {
        find: /.*\.(css|scss)$/,
        replacement: 'identity-obj-proxy',
      },
    ],
  },
  cacheDir: '.cache/vite',
  plugins: [
    tsconfigPaths({
      root: './server/',
    }),
    react({
      babel: {
        ...babelConfig,
      },
    }),
    loadCssModule({
      include: (id) => id.endsWith('scss') && !id.includes('node_modules'),
    }),
  ],
});

We run vitest like this:

VITEST_MIN_THREADS=7 VITEST_MAX_THREADS=7 yarn vitest run --silent --reporter=basic

Amount of tests:

Test Files  799 passed | 3 skipped (802)
Tests  4155 passed | 25 skipped (4180)

Here are the results:

Number of threads — Vitest Duration
1 — 1313.32s (transform 23.88s, setup 26.03s, collect 1079.13s, tests 78.64s, environment 9.98s, prepare 47.80s)
2 — 747.97s (transform 22.68s, setup 30.30s, collect 1252.21s, tests 80.26s, environment 10.46s, prepare 51.88s)
3 — 572.25s (transform 27.94s, setup 33.27s, collect 1477.32s, tests 82.24s, environment 11.18s, prepare 58.29s)
4 — 500.13s (transform 26.67s, setup 38.17s, collect 1696.73s, tests 83.31s, environment 12.11s, prepare 64.27s) 
5 — 458.09s (transform 41.15s, setup 43.74s, collect 1909.69s, tests 85.13s, environment 12.39s, prepare 67.44s)
7 — 440.87s (transform 64.18s, setup 55.04s, collect 2644.81s, tests 90.08s, environment 13.54s, prepare 77.77s)
8 — 431.75s (transform 67.33s, setup 63.98s, collect 2939.86s, tests 89.82s, environment 14.62s, prepare 84.57s)
9 — 442.80s (transform 78.38s, setup 73.08s, collect 3433.91s, tests 94.19s, environment 13.02s, prepare 95.05s)
10 — 459.41s (transform 88.00s, setup 81.83s, collect 3944.25s, tests 96.41s, environment 13.59s, prepare 102.15s)
11 — 461.70s (transform 100.57s, setup 90.47s, collect 4407.58s, tests 96.11s, environment 13.72s, prepare 109.78s)
20 — 491.86s (transform 132.02s, setup 152.44s, collect 8916.23s, tests 104.35s, environment 15.40s, prepare 124.66s)
21 — 501.20s (transform 144.72s, setup 170.01s, collect 9434.46s, tests 102.66s, environment 16.50s, prepare 150.32s)
22 — 516.64s (transform 146.94s, setup 181.19s, collect 10224.68s, tests 112.09s, environment 19.10s, prepare 163.66s)
23 — 513.61s (transform 108.65s, setup 182.26s, collect 10604.83s, tests 107.09s, environment 21.91s, prepare 141.79s)
24 — 532.76s (transform 156.73s, setup 205.62s, collect 11566.54s, tests 111.04s, environment 19.44s, prepare 142.43s)

As we can see it doesn't make sense to increase the amount of threads to more than ~8.
And as I imagine it, the bottleneck is the main vilest process that launches tinypool, but of course, it's just my assumption.
I don't know if is there any possibility to enhance it.

Also, we work around it via shards: we launch 4 shards with 3 threads in each and it takes ~3.5 minutes.

Reproduction

I can't provide repro since it's our private repo, but I can provide performance snapshots or even invite you to our repo probably without signing NDA and so on…

System Info

System:
    OS: Linux 5.10 Debian GNU/Linux 11 (bullseye) 11 (bullseye)
    CPU: (32) x64 AMD EPYC 7R13 Processor
    Memory: 59.22 GB / 61.53 GB
    Container: Yes
    Shell: 5.1.4 - /bin/bash
  Binaries:
    Node: 18.12.0 - /usr/local/bin/node
    Yarn: 3.5.1 - /usr/local/bin/yarn
    npm: 8.19.2 - /usr/local/bin/npm
 npmPackages:
    vitest: 0.34.5 => 0.34.5

Used Package Manager

yarn

Validations

@AriPerkkio
Copy link
Member

Have you done any performance profiling via --cpu-prof to see what exactly is taking the time there? E.g. node --cpu-prof --cpu-prof-dir=./profiling ./node_modules/vitest/vitest.mjs. I wonder if the Vite server running on the main thread is able to handle requests coming from that many threads.

performance problems with Vitest after migration from Jest. I am aware of eperimentalVmThreads existence, but we faced it before it was introduced.

Are you now using --experimental-vm-threads? It should make your threads behave similarly as in Jest - but it also has the same memory leaking issue.

Also, we work around it via shards: we launch 4 shards with 3 threads in each and it takes ~3.5 minutes.

Are you running these commands in parallel, e.g.:

vitest run --shard=1/4 & \
vitest run --shard=2/4 & \
vitest run --shard=3/4 & \
vitest run --shard=4/4 & \
wait # https://man7.org/linux/man-pages/man2/waitpid.2.html

4 shards with 3 threads in each

This utilizes only 4 * (3 +1) = 16 threads. In your 32 CPU machine there's still plenty of resources to use. Maybe try 4 shards with 7 threads on each?

Note that this all depends on your setup. There's no general rule that says "using more than x amount of threads makes no difference" - unless there is a performance bottleneck on main thread of course.

@7rulnik
Copy link
Contributor Author

7rulnik commented Oct 16, 2023

@AriPerkkio sorry, I missed your reply. Don't hesitate to ping me :)

I will do some profiling this week.

No, we don't use --experimental-vm-threads. I tried to migrate and did not see much of a difference in terms of speed (using the same idea with shards). But tests were much more stable, it allowed me to remove all poolMatchGlobs exceptions.
But I think it makes sense to compare it using one vitest process. Will do it too.

Yeah, I understand that we can use more threads, but we have some things in parallel to vitest too: webpack, eslint and so on. And in our case all of them kinda aligned to same time and usually finish at same time. But thanks for the recommendation anyway!

@7rulnik
Copy link
Contributor Author

7rulnik commented Oct 29, 2023

Finally, I found some time to profile.

Configuration

I decided to upgrade all dependencies and node. So what we have right now:

npx envinfo --system --npmPackages {vitest,@vitest/*,vite,@vitejs/*} --binaries --browsers
  System:
    OS: Linux 5.10 Debian GNU/Linux 11 (bullseye) 11 (bullseye)
    CPU: (32) x64 Intel(R) Xeon(R) Platinum 8375C CPU @ 2.90GHz
    Memory: 59.26 GB / 61.79 GB
    Container: Yes
    Shell: 5.1.4 - /bin/bash
  Binaries:
    Node: 21.1.0 - /usr/local/bin/node
    Yarn: 3.5.1 - /usr/local/bin/yarn
    npm: 10.2.0 - /usr/local/bin/npm
  npmPackages:
    @vitejs/plugin-react: 4.1.0 => 4.1.0 
    @vitest/ui: 0.34.6 => 0.34.6 
    vitest: 1.0.0-beta.3 => 1.0.0-beta.3

And also dropped some dependencies, switched to vmThreads (the primary reason that on our codebase it's more stable and no tests hang). So vitest config looks like this:

import { createRequire } from 'node:module';
import path from 'node:path'
import { defineConfig } from 'vitest/config';
import react from '@vitejs/plugin-react';

process.env.TZ = 'utc';
process.env.ENV = 'test';
process.env.IS_VITEST = 'true';
process.env.BABEL_ENV = 'test';
process.env.NODE_ENV = 'development';
process.env.DOTENV_CONFIG_PATH = './.env.dist';

const require = createRequire(import.meta.url);
const babelConfig = require('./babel.config');

export default defineConfig({
  test: {
    include: [
      '{client,server,scripts,test_utils,build}/**/*.{vitest,test,spec}.{ts,tsx,js}',
    ],
    pool: 'vmThreads',
    poolOptions: {
      vmThreads: {
        useAtomics: false,
        memoryLimit: '500MB',
      }
    },
    globals: true,
    setupFiles: './test_utils/vitest.setup.ts',
    testTimeout: 10_000,
    teardownTimeout: 5_000,
    server: {
      deps: {
        inline: ['@aviasales/ui', '@aviasales/analytics'],
        cacheDir: './cache/vitest/server-deps-cachedir'
      },
    },
    cache: {
      dir: '.cache/vitest/cache-dir',
    },
    poolMatchGlobs: [
      [
        '**/client/src/product/assisted/services/smartlook/__tests__/smartlook.vitest.ts',
        'threads',
      ],
      [
        '**/client/src/product/assisted/services/init/__tests__/init.vitest.ts',
        'threads',
      ],
    ],
  },
  resolve: {
    alias: [
      {
        find: /.*\.(css|scss)$/,
        replacement: 'identity-obj-proxy',
      },
      // server/tsconfig.json paths aliases
      {
        find: '&selene',
        replacement: path.resolve(__dirname, 'server/src'),
      },
      {
        find: '&platform',
        replacement: path.resolve(__dirname, 'client/src/platform'),
      },
      {
        find: '&widgets',
        replacement: path.resolve(__dirname, 'client/src/widgets'),
      },
      {
        find: '&components',
        replacement: path.resolve(__dirname, 'client/src/components'),
      },
      {
        find: '&internal',
        replacement: path.resolve(__dirname, 'client/src/internal'),
      },
      {
        find: '&services',
        replacement: path.resolve(__dirname, 'client/src/services'),
      },
      {
        find: '&product',
        replacement: path.resolve(__dirname, 'client/src/product'),
      },
      {
        find: '&test_utils',
        replacement: path.resolve(__dirname, 'test_utils'),
      },
    ],
  },
  cacheDir: '.cache/vite',
  plugins: [
    react({
      babel: {
        ...babelConfig,
        babelrc: false,
        configFile: false,
      },
    }),
  ],
});

Results:

4 shards in parallel by 3 threads in each:

VITEST_MIN_THREADS=3 VITEST_MAX_THREADS=3 yarn vitest run --shard=1/4 --silent --reporter=basic --reporter=html --outputFile.html=./vitest-reports/01/index.html
VITEST_MIN_THREADS=3 VITEST_MAX_THREADS=3 yarn vitest run --shard=2/4 --silent --reporter=basic --reporter=html --outputFile.html=./vitest-reports/02/index.html
VITEST_MIN_THREADS=3 VITEST_MAX_THREADS=3 yarn vitest run --shard=3/4 --silent --reporter=basic --reporter=html --outputFile.html=./vitest-reports/03/index.html
VITEST_MIN_THREADS=3 VITEST_MAX_THREADS=3 yarn vitest run --shard=4/4 --silent --reporter=basic --reporter=html --outputFile.html=./vitest-reports/04/index.html
125.00s (transform 14.74s, setup 5.21s, collect 343.05s, tests 11.34s, environment 1.11s, prepare 11.36s)
119.05s (transform 54.71s, setup 5.13s, collect 319.42s, tests 22.10s, environment 791ms, prepare 10.76s)
114.76s (transform 24.02s, setup 4.77s, collect 301.13s, tests 17.99s, environment 905ms, prepare 10.48s)
125.38s (transform 23.66s, setup 5.68s, collect 307.52s, tests 44.41s, environment 891ms, prepare 10.24s)

12 threads

VITEST_MIN_THREADS=12 VITEST_MAX_THREADS=12 node ./node_modules/vitest/vitest.mjs run --silent --reporter=basic --reporter=html --reporter=html --outputFile.html=./vitest-reports/01/index.html
165.74s (transform 64.41s, setup 23.55s, collect 1726.11s, tests 94.15s, environment 3.25s, prepare 45.03s)

24 threads

VITEST_MIN_THREADS=24 VITEST_MAX_THREADS=24 node ./node_modules/vitest/vitest.mjs run --silent --reporter=basic --reporter=html --reporter=html --outputFile.html=./vitest-reports/01/index.html

153.72s (transform 82.56s, setup 37.77s, collect 3216.90s, tests 98.16s, environment 4.61s, prepare 61.28s)

I captured some profiles and mentioned that a lot of time was spend for garbage collection. I tried to increase --max-semi-space-size (see nodejs/node#47277) to 64mb and 128mb.

It led to time decreasion spent on GC from 16s to 8s (64mb) and 6s (128mb). It didn't make any diffrenece for total execution time, but on node v18.12.0 it allowed me to run more threads. Without this option vitest with 24 threads failed some tests with [birpc] timeout (https://github.com/antfu/birpc/blob/main/src/index.ts#L183)

I didn't get meaningful insights from flamegraphs. But it's obvious that a lot of time spent on file transformation. Is it possible that shards allow to parallel transformations and it leads to faster execution?

image

There are cpu profiles:
https://drive.google.com/file/d/15I42Mrpst9yHBgqKkDPv1ztPt13NRuMa/view?usp=drive_link

@sheremet-va
Copy link
Member

Is it possible that shards allow to parallel transformations and it leads to faster execution?

Each shard is a separate process with its own module graph so yes, it's running in parallel without affecting each other much.

By default, I would expect Vitest to have a limit after which increasing the number of cores doesn't make a lot of sense because the main process is just overloaded by requests from other threads. Looks like sharding alleviates this problem.

@7rulnik
Copy link
Contributor Author

7rulnik commented Nov 14, 2023

So there is nothing we can do?

If so, I think we should mention in the documentation that shards could speed up tests. I think it makes sense to create a separate section about performance on website.

Also, it would be nice to have some API to concat report artifacts like JUnit, JSON, and HTML which is a separate issue indeed. If you don't mind about this idea I can create issue for it and send PR.

@sheremet-va
Copy link
Member

sheremet-va commented Nov 15, 2023

So there is nothing we can do?

From the perspective of Vitest, we provide all the tools already but don't document it enough.

Also, it would be nice to have some API to concat report artifacts like JUnit, JSON, and HTML which is a separate issue indeed. If you don't mind about this idea I can create issue for it and send PR.

I think there is either an open PR or discussion about this, but yes it should be fine to open a new issue. I think playwright uses blob reporter for this.

If so, I think we should mention in the documentation that shards could speed up tests. I think it makes sense to create a separate section about performance on website.

Yes, we should probably have a separate guide page about sharding. Currently, it's only mentioned in the config docs with the assumption that people already know what to do with it.

@sheremet-va sheremet-va moved this to Discussing in Team Board Feb 12, 2024
@sheremet-va sheremet-va moved this from Discussing to P2 - 3 in Team Board Feb 12, 2024
@sheremet-va sheremet-va moved this from P2 - 3 to P2 - 2 in Team Board Feb 12, 2024
@7rulnik 7rulnik changed the title The performance of threads doesn't scale much after 8 threads [Docs] Explain how to use sharding Apr 21, 2024
@7rulnik
Copy link
Contributor Author

7rulnik commented Apr 21, 2024

So I renamed this issue a bit. And yeah, there is the issue about reports concatenation #5125

@AriPerkkio
Copy link
Member

Let's add documentation in #5736.

@github-actions github-actions bot locked and limited conversation to collaborators Jun 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants