From 237ded403e6012a48281f4572eab0c8eafe55b3f Mon Sep 17 00:00:00 2001 From: Gleb Bahmutov Date: Thu, 15 Feb 2024 17:59:22 -0500 Subject: [PATCH] fix: solve the performance problem (#220) By exiting after upload finishes to stop dangling promises from preventing the Node process finish --- .github/workflows/example-performance.yml | 27 +++++++++++++ .prettierignore | 1 + dist/index.js | 48 +++++++++++++++++++---- index.js | 48 +++++++++++++++++++---- package-lock.json | 14 +++---- package.json | 2 +- 6 files changed, 116 insertions(+), 24 deletions(-) create mode 100644 .github/workflows/example-performance.yml diff --git a/.github/workflows/example-performance.yml b/.github/workflows/example-performance.yml new file mode 100644 index 0000000..f2fa51b --- /dev/null +++ b/.github/workflows/example-performance.yml @@ -0,0 +1,27 @@ +name: example-performance +on: [pull_request] +jobs: + # cache using npm-install + cache1: + runs-on: ubuntu-22.04 + steps: + - uses: actions/checkout@v4 + # look at fast this action installs a single dependency + - uses: bahmutov/npm-install@HEAD + with: + working-directory: examples/basic + install-command: 'npm install chalk' + cache-key-prefix: chalk8 + + # cache using https://github.com/actions/cache + cache2: + runs-on: ubuntu-20.04 + steps: + - uses: actions/checkout@v4 + - name: Caching + uses: actions/cache@v4 + with: + path: ~/.npm + key: chalk-v2 + - name: Install just chalk + run: npm install chalk diff --git a/.prettierignore b/.prettierignore index 7bc86a5..490dc00 100644 --- a/.prettierignore +++ b/.prettierignore @@ -5,3 +5,4 @@ yarn.lock .gitignore .prettierignore .npmrc +.nvmrc diff --git a/dist/index.js b/dist/index.js index 0fab339..a352ed9 100644 --- a/dist/index.js +++ b/dist/index.js @@ -64,8 +64,17 @@ const saveCachedNpm = npmCache => { console.log('saving NPM modules under key %s', npmCache.primaryKey) console.log('input paths: %o', npmCache.inputPaths) + const started = +new Date() return cache .saveCache(npmCache.inputPaths, npmCache.primaryKey) + .then(() => { + const finished = +new Date() + console.log( + 'npm cache saved for key %s, took %dms', + npmCache.primaryKey, + finished - started + ) + }) .catch(err => { // don't throw an error if cache already exists, which may happen due to // race conditions @@ -304,14 +313,34 @@ const npmInstallAction = async () => { const installCommand = core.getInput('install-command') - for (const workingDirectory of workingDirectories) { - await api.utils.installInOneFolder({ - usePackageLock, - useRollingCache, - workingDirectory, - installCommand, - cachePrefix - }) + try { + for (const workingDirectory of workingDirectories) { + const started = +new Date() + await api.utils.installInOneFolder({ + usePackageLock, + useRollingCache, + workingDirectory, + installCommand, + cachePrefix + }) + + const finished = +new Date() + core.debug( + `installing in ${workingDirectory} took ${finished - started}ms` + ) + } + + // node will stay alive if any promises are not resolved, + // which is a possibility if HTTP requests are dangling + // due to retries or timeouts. We know that if we got here + // that all promises that we care about have successfully + // resolved, so simply exit with success. + // From: https://github.com/actions/cache/blob/a2ed59d39b352305bdd2f628719a53b2cc4f9613/src/saveImpl.ts#L96 + process.exit(0) + } catch (err) { + console.error(err) + core.setFailed(err.message) + process.exit(1) } } @@ -336,9 +365,12 @@ module.exports = api // @ts-ignore if (!module.parent) { console.log('running npm-install GitHub Action') + const started = +new Date() npmInstallAction() .then(() => { console.log('all done, exiting') + const finished = +new Date() + core.debug(`npm-install took ${finished - started}ms`) }) .catch(error => { console.log(error) diff --git a/index.js b/index.js index a69b6f9..ef369b2 100644 --- a/index.js +++ b/index.js @@ -57,8 +57,17 @@ const saveCachedNpm = npmCache => { console.log('saving NPM modules under key %s', npmCache.primaryKey) console.log('input paths: %o', npmCache.inputPaths) + const started = +new Date() return cache .saveCache(npmCache.inputPaths, npmCache.primaryKey) + .then(() => { + const finished = +new Date() + console.log( + 'npm cache saved for key %s, took %dms', + npmCache.primaryKey, + finished - started + ) + }) .catch(err => { // don't throw an error if cache already exists, which may happen due to // race conditions @@ -297,14 +306,34 @@ const npmInstallAction = async () => { const installCommand = core.getInput('install-command') - for (const workingDirectory of workingDirectories) { - await api.utils.installInOneFolder({ - usePackageLock, - useRollingCache, - workingDirectory, - installCommand, - cachePrefix - }) + try { + for (const workingDirectory of workingDirectories) { + const started = +new Date() + await api.utils.installInOneFolder({ + usePackageLock, + useRollingCache, + workingDirectory, + installCommand, + cachePrefix + }) + + const finished = +new Date() + core.debug( + `installing in ${workingDirectory} took ${finished - started}ms` + ) + } + + // node will stay alive if any promises are not resolved, + // which is a possibility if HTTP requests are dangling + // due to retries or timeouts. We know that if we got here + // that all promises that we care about have successfully + // resolved, so simply exit with success. + // From: https://github.com/actions/cache/blob/a2ed59d39b352305bdd2f628719a53b2cc4f9613/src/saveImpl.ts#L96 + process.exit(0) + } catch (err) { + console.error(err) + core.setFailed(err.message) + process.exit(1) } } @@ -329,9 +358,12 @@ module.exports = api // @ts-ignore if (!module.parent) { console.log('running npm-install GitHub Action') + const started = +new Date() npmInstallAction() .then(() => { console.log('all done, exiting') + const finished = +new Date() + core.debug(`npm-install took ${finished - started}ms`) }) .catch(error => { console.log(error) diff --git a/package-lock.json b/package-lock.json index 99862f7..68ae0bc 100644 --- a/package-lock.json +++ b/package-lock.json @@ -19,7 +19,7 @@ }, "devDependencies": { "@types/node": "^12.0.4", - "@vercel/ncc": "0.36.1", + "@vercel/ncc": "0.38.1", "chai": "4.2.0", "husky": "3.0.9", "mocha": "7.0.1", @@ -1223,9 +1223,9 @@ } }, "node_modules/@vercel/ncc": { - "version": "0.36.1", - "resolved": "https://registry.npmjs.org/@vercel/ncc/-/ncc-0.36.1.tgz", - "integrity": "sha512-S4cL7Taa9yb5qbv+6wLgiKVZ03Qfkc4jGRuiUQMQ8HGBD5pcNRnHeYM33zBvJE4/zJGjJJ8GScB+WmTsn9mORw==", + "version": "0.38.1", + "resolved": "https://registry.npmjs.org/@vercel/ncc/-/ncc-0.38.1.tgz", + "integrity": "sha512-IBBb+iI2NLu4VQn3Vwldyi2QwaXt5+hTyh58ggAMoCGE6DJmPvwL3KPBWcJl1m9LYPChBLE980Jw+CS4Wokqxw==", "dev": true, "bin": { "ncc": "dist/ncc/cli.js" @@ -10697,9 +10697,9 @@ } }, "@vercel/ncc": { - "version": "0.36.1", - "resolved": "https://registry.npmjs.org/@vercel/ncc/-/ncc-0.36.1.tgz", - "integrity": "sha512-S4cL7Taa9yb5qbv+6wLgiKVZ03Qfkc4jGRuiUQMQ8HGBD5pcNRnHeYM33zBvJE4/zJGjJJ8GScB+WmTsn9mORw==", + "version": "0.38.1", + "resolved": "https://registry.npmjs.org/@vercel/ncc/-/ncc-0.38.1.tgz", + "integrity": "sha512-IBBb+iI2NLu4VQn3Vwldyi2QwaXt5+hTyh58ggAMoCGE6DJmPvwL3KPBWcJl1m9LYPChBLE980Jw+CS4Wokqxw==", "dev": true }, "abort-controller": { diff --git a/package.json b/package.json index ecf57f8..b0e049e 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,7 @@ }, "devDependencies": { "@types/node": "^12.0.4", - "@vercel/ncc": "0.36.1", + "@vercel/ncc": "0.38.1", "chai": "4.2.0", "husky": "3.0.9", "mocha": "7.0.1",