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
feat: various v8 snapshot improvements #24909
Changes from all commits
3f3e308
7d65a96
fcfa789
90c96c6
5493177
474b3a4
e6cde79
ac5ebdc
25b870c
968e726
51426dd
c071402
8ab76e6
b4e2261
df39189
239a953
b4f8119
b77cf7c
e2dd80f
dc0332a
6b5185c
5e6f47d
42f8935
0c2dfae
d48caa8
528d91b
9a4157e
35678ef
cb7e3b2
1a4c521
1c1ce89
76a18b6
1720cb8
3ea52dc
250e9f8
9ba3016
35c8b6f
401fbdc
d3c832a
eb0dcfe
f27b687
b199681
bd533d3
8d2598a
39d02d6
ce8c74a
fe8b6cd
bb1390e
aa6b359
4187ff6
ab63313
6751287
bebcd8a
30660b0
5c1f981
adadd79
5f3a735
12a5753
5442930
4e8d8e3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
# Bump this version to force CI to re-create the cache from scratch. | ||
|
||
10-31-22 | ||
12-01-22 |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,13 +3,8 @@ if (process.env.CYPRESS_INTERNAL_ENV === 'production') { | |
throw new Error(`${__filename} should only run outside of prod`) | ||
} | ||
|
||
if (require.name !== 'customRequire') { | ||
// Purposefully make this a dynamic require so that it doesn't have the potential to get picked up by snapshotting mechanism | ||
const hook = './hook' | ||
const { hookRequire } = require('@packages/server/hook-require') | ||
|
||
const { hookRequire } = require(`@packages/server/${hook}-require`) | ||
|
||
hookRequire(true) | ||
} | ||
hookRequire({ forceTypeScript: true }) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. how do we know to force typescript? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This file never runs in prod (see line 2). It only runs in development where we still need to force typescript compilation on the fly. |
||
|
||
require('../lib/threads/worker.ts') |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,18 +1,19 @@ | ||
const { initializeStartTime } = require('./lib/util/performance_benchmark') | ||
|
||
const run = async () => { | ||
initializeStartTime() | ||
const startCypress = async () => { | ||
try { | ||
initializeStartTime() | ||
|
||
if (require.name !== 'customRequire') { | ||
// Purposefully make this a dynamic require so that it doesn't have the potential to get picked up by snapshotting mechanism | ||
const hook = './hook' | ||
const { hookRequire } = require('./hook-require') | ||
|
||
const { hookRequire } = require(`${hook}-require`) | ||
hookRequire({ forceTypeScript: false }) | ||
|
||
hookRequire(false) | ||
await require('./start-cypress') | ||
} catch (error) { | ||
// eslint-disable-next-line no-console | ||
console.error(error) | ||
process.exit(1) | ||
} | ||
|
||
await require('./server-entry') | ||
} | ||
|
||
module.exports = run() | ||
module.exports = startCypress() |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
require('./start-cypress') |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -6,6 +6,7 @@ const esbuild = require('esbuild') | |||||
const snapshotMetadata = require('@tooling/v8-snapshot/cache/prod-darwin/snapshot-meta.cache.json') | ||||||
const tempDir = require('temp-dir') | ||||||
const workingDir = path.join(tempDir, 'binary-cleanup-workdir') | ||||||
const bytenode = require('bytenode') | ||||||
|
||||||
fs.ensureDirSync(workingDir) | ||||||
|
||||||
|
@@ -39,8 +40,6 @@ async function removeEmptyDirectories (directory) { | |||||
const getDependencyPathsToKeep = async (buildAppDir) => { | ||||||
const unixBuildAppDir = buildAppDir.split(path.sep).join(path.posix.sep) | ||||||
const startingEntryPoints = [ | ||||||
'packages/server/index.js', | ||||||
'packages/server/hook-require.js', | ||||||
'packages/server/lib/plugins/child/require_async_child.js', | ||||||
'packages/server/lib/plugins/child/register_ts_node.js', | ||||||
'packages/rewriter/lib/threads/worker.js', | ||||||
|
@@ -81,7 +80,7 @@ const getDependencyPathsToKeep = async (buildAppDir) => { | |||||
absWorkingDir: unixBuildAppDir, | ||||||
external: [ | ||||||
'./transpile-ts', | ||||||
'./server-entry', | ||||||
'./start-cypress', | ||||||
'fsevents', | ||||||
'pnpapi', | ||||||
'@swc/core', | ||||||
|
@@ -111,18 +110,63 @@ const getDependencyPathsToKeep = async (buildAppDir) => { | |||||
}) | ||||||
} | ||||||
|
||||||
return [...Object.keys(esbuildResult.metafile.inputs), ...entryPoints] | ||||||
return [...Object.keys(esbuildResult.metafile.inputs), ...entryPoints, 'package.json'] | ||||||
} | ||||||
|
||||||
const cleanup = async (buildAppDir) => { | ||||||
// 1. Retrieve all dependencies that still need to be kept in the binary. In theory, we could use the bundles generated here as single files within the binary, | ||||||
// but for now, we just track on the dependencies that get pulled in | ||||||
const keptDependencies = [...await getDependencyPathsToKeep(buildAppDir), 'package.json', 'packages/server/server-entry.js'] | ||||||
const createServerEntryPointBundle = async (buildAppDir) => { | ||||||
const unixBuildAppDir = buildAppDir.split(path.sep).join(path.posix.sep) | ||||||
const entryPoints = [path.join(unixBuildAppDir, 'packages/server/index.js')] | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am trying to limit node internals references as much as I possibly can. See: and |
||||||
// Build the binary entry point ignoring anything that happens in start-cypress since that will be in the v8 snapshot | ||||||
const esbuildResult = await esbuild.build({ | ||||||
ryanthemanuel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
entryPoints, | ||||||
bundle: true, | ||||||
outdir: workingDir, | ||||||
platform: 'node', | ||||||
metafile: true, | ||||||
absWorkingDir: unixBuildAppDir, | ||||||
external: [ | ||||||
'./transpile-ts', | ||||||
'./start-cypress', | ||||||
], | ||||||
}) | ||||||
|
||||||
console.log(`copying server entry point bundle from ${path.join(workingDir, 'index.js')} to ${path.join(buildAppDir, 'packages', 'server', 'index.js')}`) | ||||||
|
||||||
await fs.copy(path.join(workingDir, 'index.js'), path.join(buildAppDir, 'packages', 'server', 'index.js')) | ||||||
|
||||||
console.log(`compiling server entry point bundle to ${path.join(buildAppDir, 'packages', 'server', 'index.jsc')}`) | ||||||
|
||||||
// Use bytenode to compile the entry point bundle. This will save time on the v8 compile step and ensure the integrity of the entry point | ||||||
await bytenode.compileFile({ | ||||||
ryanthemanuel marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
filename: path.join(buildAppDir, 'packages', 'server', 'index.js'), | ||||||
output: path.join(buildAppDir, 'packages', 'server', 'index.jsc'), | ||||||
electron: true, | ||||||
}) | ||||||
|
||||||
// Convert these inputs to a relative file path. Note that these paths are posix paths. | ||||||
return [...Object.keys(esbuildResult.metafile.inputs)].map((input) => `./${input}`) | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this be a hard-coded separator? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, the way these files are read in assumes "require" syntax not platform specific syntax. |
||||||
} | ||||||
|
||||||
const buildEntryPointAndCleanup = async (buildAppDir) => { | ||||||
const [keptDependencies, serverEntryPointBundleDependencies] = await Promise.all([ | ||||||
// 1. Retrieve all dependencies that still need to be kept in the binary. In theory, we could use the bundles generated here as single files within the binary, | ||||||
// but for now, we just track on the dependencies that get pulled in | ||||||
getDependencyPathsToKeep(buildAppDir), | ||||||
// 2. Create a bundle for the server entry point. This will be used to start the server in the binary. It returns the dependencies that are pulled in by this bundle that potentially can now be removed | ||||||
createServerEntryPointBundle(buildAppDir), | ||||||
]) | ||||||
|
||||||
// 3. Gather the dependencies that could potentially be removed from the binary due to being in the snapshot or in the entry point bundle | ||||||
const potentiallyRemovedDependencies = [ | ||||||
...snapshotMetadata.healthy, | ||||||
...snapshotMetadata.deferred, | ||||||
...snapshotMetadata.norewrite, | ||||||
...serverEntryPointBundleDependencies, | ||||||
] | ||||||
|
||||||
// 2. Gather the dependencies that could potentially be removed from the binary due to being in the snapshot | ||||||
const potentiallyRemovedDependencies = [...snapshotMetadata.healthy, ...snapshotMetadata.deferred, ...snapshotMetadata.norewrite] | ||||||
console.log(`potentially removing ${potentiallyRemovedDependencies.length} dependencies`) | ||||||
|
||||||
// 3. Remove all dependencies that are in the snapshot but not in the list of kept dependencies from the binary | ||||||
// 4. Remove all dependencies that are in the snapshot but not in the list of kept dependencies from the binary | ||||||
await Promise.all(potentiallyRemovedDependencies.map(async (dependency) => { | ||||||
const typeScriptlessDependency = dependency.replace(/\.ts$/, '.js') | ||||||
|
||||||
|
@@ -132,10 +176,10 @@ const cleanup = async (buildAppDir) => { | |||||
} | ||||||
})) | ||||||
|
||||||
// 4. Consolidate dependencies that are safe to consolidate (`lodash` and `bluebird`) | ||||||
// 5. Consolidate dependencies that are safe to consolidate (`lodash` and `bluebird`) | ||||||
await consolidateDeps({ projectBaseDir: buildAppDir }) | ||||||
|
||||||
// 5. Remove various unnecessary files from the binary to further clean things up. Likely, there is additional work that can be done here | ||||||
// 6. Remove various unnecessary files from the binary to further clean things up. Likely, there is additional work that can be done here | ||||||
await del([ | ||||||
// Remove test files | ||||||
path.join(buildAppDir, '**', 'test'), | ||||||
|
@@ -187,10 +231,10 @@ const cleanup = async (buildAppDir) => { | |||||
path.join(buildAppDir, '**', 'yarn.lock'), | ||||||
], { force: true }) | ||||||
|
||||||
// 6. Remove any empty directories as a result of the rest of the cleanup | ||||||
// 7. Remove any empty directories as a result of the rest of the cleanup | ||||||
await removeEmptyDirectories(buildAppDir) | ||||||
} | ||||||
|
||||||
module.exports = { | ||||||
cleanup, | ||||||
buildEntryPointAndCleanup, | ||||||
} |
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
@@ -0,0 +1,21 @@ | ||||||
const Module = require('module') | ||||||
const path = require('path') | ||||||
|
||||||
process.env.CYPRESS_INTERNAL_ENV = process.env.CYPRESS_INTERNAL_ENV || 'production' | ||||||
try { | ||||||
require('./node_modules/bytenode/lib/index.js') | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is index.js not the default export?
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm actually trying to purposefully reference the exact file here. I don't anyone to be able to use any kind of module loading weirdness to reference a different file (may not be possible, but just in case). |
||||||
const filename = path.join(__dirname, 'packages', 'server', 'index.jsc') | ||||||
const dirname = path.dirname(filename) | ||||||
|
||||||
Module._extensions['.jsc']({ | ||||||
require: module.require, | ||||||
id: filename, | ||||||
filename, | ||||||
loaded: false, | ||||||
path: dirname, | ||||||
paths: Module._nodeModulePaths(dirname), | ||||||
}, filename) | ||||||
} catch (error) { | ||||||
BlueWinds marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
console.error(error) | ||||||
process.exit(1) | ||||||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We're doing a lot in the binary build now.
xlarge
is needed to not run out of memory.