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

perf: improve performance of forks pool #5592

Merged
merged 10 commits into from
May 1, 2024
29 changes: 25 additions & 4 deletions packages/vite-node/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,11 @@ export class ViteNodeServer {
web: new Map<string, Promise<TransformResult | null | undefined>>(),
}

private durations = {
ssr: new Map<string, number[]>(),
web: new Map<string, number[]>(),
}

private existingOptimizedDeps = new Set<string>()

fetchCaches = {
Expand Down Expand Up @@ -102,6 +107,12 @@ export class ViteNodeServer {
return shouldExternalize(id, this.options.deps, this.externalizeCache)
}

public getTotalDuration() {
const ssrDurations = [...this.durations.ssr.values()].flat()
const webDurations = [...this.durations.web.values()].flat()
return [...ssrDurations, ...webDurations].reduce((a, b) => a + b, 0)
}

private async ensureExists(id: string): Promise<boolean> {
if (this.existingOptimizedDeps.has(id))
return true
Expand Down Expand Up @@ -138,16 +149,20 @@ export class ViteNodeServer {
}

async fetchModule(id: string, transformMode?: 'web' | 'ssr'): Promise<FetchResult> {
const moduleId = normalizeModuleId(id)
const mode = transformMode || this.getTransformMode(id)
return this.fetchResult(id, mode)
.then((r) => {
return this.options.sourcemap !== true ? { ...r, map: undefined } : r
})
}

async fetchResult(id: string, mode: 'web' | 'ssr') {
const moduleId = normalizeModuleId(id)
this.assertMode(mode)
const promiseMap = this.fetchPromiseMap[mode]
// reuse transform for concurrent requests
if (!promiseMap.has(moduleId)) {
promiseMap.set(moduleId, this._fetchModule(moduleId, mode)
.then((r) => {
return this.options.sourcemap !== true ? { ...r, map: undefined } : r
})
.finally(() => {
promiseMap.delete(moduleId)
}))
Expand Down Expand Up @@ -268,6 +283,12 @@ export class ViteNodeServer {
result,
}

const durations = this.durations[transformMode].get(filePath) || []
this.durations[transformMode].set(
filePath,
[...durations, duration ?? 0],
)

this.fetchCaches[transformMode].set(filePath, cacheEntry)
this.fetchCache.set(filePath, cacheEntry)

Expand Down
8 changes: 7 additions & 1 deletion packages/vitest/src/integrations/env/loader.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { readFileSync } from 'node:fs'
import { normalize, resolve } from 'pathe'
import { ViteNodeRunner } from 'vite-node/client'
import type { ViteNodeRunnerOptions } from 'vite-node'
Expand Down Expand Up @@ -26,7 +27,12 @@ export async function loadEnvironment(ctx: ContextRPC, rpc: WorkerRPC): Promise<
return environments[name]
const loader = await createEnvironmentLoader({
root: ctx.config.root,
fetchModule: id => rpc.fetch(id, 'ssr'),
fetchModule: async (id) => {
const result = await rpc.fetch(id, 'ssr')
if (result.id)
return { code: readFileSync(result.id, 'utf-8') }
Copy link

Choose a reason for hiding this comment

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

why using sync IO in async context? 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

because it’s faster. we rarely import multiple modules at the same time, so nothing is executed in parallel most of the time

return result
},
resolveId: (id, importer) => rpc.resolveId(id, importer, 'ssr'),
})
const root = loader.root
Expand Down
32 changes: 30 additions & 2 deletions packages/vitest/src/node/pools/rpc.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
import { mkdir, writeFile } from 'node:fs/promises'
import type { RawSourceMap } from 'vite-node'
import { join } from 'pathe'
import type { RuntimeRPC } from '../../types'
import type { WorkspaceProject } from '../workspace'

const created = new Set()
const promises = new Map<string, Promise<void>>()

export function createMethodsRPC(project: WorkspaceProject): RuntimeRPC {
const ctx = project.ctx
return {
Expand All @@ -20,8 +25,31 @@ export function createMethodsRPC(project: WorkspaceProject): RuntimeRPC {
const r = await project.vitenode.transformRequest(id)
return r?.map as RawSourceMap | undefined
},
fetch(id, transformMode) {
return project.vitenode.fetchModule(id, transformMode)
async fetch(id, transformMode) {
AriPerkkio marked this conversation as resolved.
Show resolved Hide resolved
const result = await project.vitenode.fetchResult(id, transformMode)
const code = result.code
if (result.externalize)
return result
if ('id' in result)
return { id: result.id as string }

if (!code)
throw new Error(`Failed to fetch module ${id}`)

const dir = join(project.tmpDir, transformMode)
const tmp = join(dir, id.replace(/[/\\?%*:|"<>]/g, '_').replace('\0', '__x00__'))
Comment on lines +39 to +40
Copy link
Member

Choose a reason for hiding this comment

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

I think this is causing too long filenames:

https://github.com/vitest-dev/vitest-ecosystem-ci/actions/runs/8911833491/job/24473995666

⎯⎯⎯⎯⎯⎯ Unhandled Error ⎯⎯⎯⎯⎯⎯⎯
  Error: ENAMETOOLONG: name too long, open '/tmp/a5LDqI7rqUo2IYVisuaPM/ssr/_home_runner_work_vitest-ecosystem-ci_vitest-ecosystem-ci_workspace_elk_elk_node_modules_.pnpm_vitest-environment-nuxt@1.0.0_@vitest+ui@1.5.3_@vue+test-utils@2.4.5_h3@1.11.1_happy-dom@10.5_ftcywtwlwumcv752fah2mtnl3i_node_modules_vitest-environment-nuxt_index.mjs'

if (promises.has(tmp)) {
await promises.get(tmp)
return { id: tmp }
}
if (!created.has(dir)) {
await mkdir(dir, { recursive: true })
created.add(dir)
}
promises.set(tmp, writeFile(tmp, code, 'utf-8').finally(() => promises.delete(tmp)))
await promises.get(tmp)
Object.assign(result, { id: tmp })
return { id: tmp }
AriPerkkio marked this conversation as resolved.
Show resolved Hide resolved
},
resolveId(id, importer, transformMode) {
return project.vitenode.resolveId(id, importer, transformMode)
Expand Down
2 changes: 1 addition & 1 deletion packages/vitest/src/node/reporters/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ export abstract class BaseReporter implements Reporter {
const setupTime = files.reduce((acc, test) => acc + Math.max(0, test.setupDuration || 0), 0)
const testsTime = files.reduce((acc, test) => acc + Math.max(0, test.result?.duration || 0), 0)
const transformTime = this.ctx.projects
.flatMap(w => Array.from(w.vitenode.fetchCache.values()).map(i => i.duration || 0))
.flatMap(w => w.vitenode.getTotalDuration())
.reduce((a, b) => a + b, 0)
const environmentTime = files.reduce((acc, file) => acc + Math.max(0, file.environmentLoad || 0), 0)
const prepareTime = files.reduce((acc, file) => acc + Math.max(0, file.prepareDuration || 0), 0)
Expand Down
15 changes: 14 additions & 1 deletion packages/vitest/src/node/workspace.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,6 @@
import { promises as fs } from 'node:fs'
import { rm } from 'node:fs/promises'
import { tmpdir } from 'node:os'
import fg from 'fast-glob'
import mm from 'micromatch'
import { dirname, isAbsolute, join, relative, resolve, toNamespacedPath } from 'pathe'
Expand All @@ -8,10 +10,10 @@ import { ViteNodeServer } from 'vite-node/server'
import c from 'picocolors'
import { createBrowserServer } from '../integrations/browser/server'
import type { ProvidedContext, ResolvedConfig, UserConfig, UserWorkspaceConfig, Vitest } from '../types'
import { deepMerge } from '../utils'
import type { Typechecker } from '../typecheck/typechecker'
import type { BrowserProvider } from '../types/browser'
import { getBrowserProvider } from '../integrations/browser'
import { deepMerge, nanoid } from '../utils/base'
import { isBrowserEnabled, resolveConfig } from './config'
import { WorkspaceVitestPlugin } from './plugins/workspace'
import { createViteServer } from './vite'
Expand Down Expand Up @@ -78,6 +80,9 @@ export class WorkspaceProject {

testFilesList: string[] | null = null

public readonly id = nanoid()
public readonly tmpDir = join(tmpdir(), this.id)

private _globalSetups: GlobalSetupFile[] | undefined
private _provided: ProvidedContext = {} as any

Expand Down Expand Up @@ -402,11 +407,19 @@ export class WorkspaceProject {
this.server.close(),
this.typechecker?.stop(),
this.browser?.close(),
this.clearTmpDir(),
].filter(Boolean)).then(() => this._provided = {} as any)
}
return this.closingPromise
}

private async clearTmpDir() {
try {
await rm(this.tmpDir, { force: true, recursive: true })
}
catch {}
}

async initBrowserProvider() {
if (!this.isBrowserEnabled())
return
Expand Down
8 changes: 7 additions & 1 deletion packages/vitest/src/runtime/execute.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import vm from 'node:vm'
import { pathToFileURL } from 'node:url'
import { readFileSync } from 'node:fs'
import type { ModuleCacheMap } from 'vite-node/client'
import { DEFAULT_REQUEST_STUBS, ViteNodeRunner } from 'vite-node/client'
import { isInternalRequest, isNodeBuiltin, isPrimitive, toFilePath } from 'vite-node/utils'
Expand Down Expand Up @@ -104,7 +105,12 @@ export async function startVitestExecutor(options: ContextExecutorOptions) {
return { externalize: id }
}

return rpc().fetch(id, getTransformMode())
const result = await rpc().fetch(id, getTransformMode())
if (result.id && !result.externalize) {
const code = readFileSync(result.id, 'utf-8')
return { code }
}
return result
},
resolveId(id, importer) {
return rpc().resolveId(id, importer, getTransformMode())
Expand Down
5 changes: 4 additions & 1 deletion packages/vitest/src/types/rpc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import type { AfterSuiteRunMeta } from './worker'
type TransformMode = 'web' | 'ssr'

export interface RuntimeRPC {
fetch: (id: string, environment: TransformMode) => Promise<FetchResult>
fetch: (id: string, environment: TransformMode) => Promise<{
externalize?: string
id?: string
}>
transform: (id: string, environment: TransformMode) => Promise<FetchResult>
resolveId: (id: string, importer: string | undefined, environment: TransformMode) => Promise<ViteNodeResolveId | null>
getSourceMap: (id: string, force?: boolean) => Promise<RawSourceMap | undefined>
Expand Down
11 changes: 11 additions & 0 deletions packages/vitest/src/utils/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,3 +169,14 @@ export function escapeRegExp(s: string) {
export function wildcardPatternToRegExp(pattern: string): RegExp {
return new RegExp(`^${pattern.split('*').map(escapeRegExp).join('.*')}$`, 'i')
}

// port from nanoid
// https://github.com/ai/nanoid
const urlAlphabet
= 'useandom-26T198340PX75pxJACKVERYMINDBUSHWOLF_GQZbfghjklqvwyzrict'
export function nanoid(size = 21) {
let id = ''
let i = size
while (i--) id += urlAlphabet[(Math.random() * 64) | 0]
return id
}
10 changes: 8 additions & 2 deletions packages/web-worker/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { readFileSync } from 'node:fs'
import type { WorkerGlobalState } from 'vitest'
import ponyfillStructuredClone from '@ungap/structured-clone'
import createDebug from 'debug'
Expand Down Expand Up @@ -65,8 +66,13 @@ export function getRunnerOptions(): any {
const { config, rpc, mockMap, moduleCache } = state

return {
fetchModule(id: string) {
return rpc.fetch(id, 'web')
async fetchModule(id: string) {
const result = await rpc.fetch(id, 'web')
if (result.id && !result.externalize) {
const code = readFileSync(result.id, 'utf-8')
return { code }
}
return result
},
resolveId(id: string, importer?: string) {
return rpc.resolveId(id, importer, 'web')
Expand Down
19 changes: 15 additions & 4 deletions pnpm-lock.yaml

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