Skip to content

Commit

Permalink
fix: solve the performance problem (#220)
Browse files Browse the repository at this point in the history
By exiting after upload finishes to stop dangling promises from preventing the Node process finish
  • Loading branch information
bahmutov authored Feb 15, 2024
1 parent 99e7d72 commit 237ded4
Show file tree
Hide file tree
Showing 6 changed files with 116 additions and 24 deletions.
27 changes: 27 additions & 0 deletions .github/workflows/example-performance.yml
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,4 @@ yarn.lock
.gitignore
.prettierignore
.npmrc
.nvmrc
48 changes: 40 additions & 8 deletions dist/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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)
Expand Down
48 changes: 40 additions & 8 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
}
}

Expand All @@ -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)
Expand Down
14 changes: 7 additions & 7 deletions package-lock.json

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

2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down

0 comments on commit 237ded4

Please sign in to comment.