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

Optimize internal getStats() call #762

Closed
RyanZim opened this issue Feb 13, 2020 · 1 comment · Fixed by #764
Closed

Optimize internal getStats() call #762

RyanZim opened this issue Feb 13, 2020 · 1 comment · Fixed by #764
Assignees
Milestone

Comments

@RyanZim
Copy link
Collaborator

RyanZim commented Feb 13, 2020

This function makes two async stat calls, one after the other:

function getStats (src, dest, cb) {
if (nodeSupportsBigInt()) {
fs.stat(src, { bigint: true }, (err, srcStat) => {
if (err) return cb(err)
fs.stat(dest, { bigint: true }, (err, destStat) => {
if (err) {
if (err.code === 'ENOENT') return cb(null, { srcStat, destStat: null })
return cb(err)
}
return cb(null, { srcStat, destStat })
})
})
} else {
fs.stat(src, (err, srcStat) => {
if (err) return cb(err)
fs.stat(dest, (err, destStat) => {
if (err) {
if (err.code === 'ENOENT') return cb(null, { srcStat, destStat: null })
return cb(err)
}
return cb(null, { srcStat, destStat })
})
})
}
}

We could optimize this by making both calls at the same time, then awaiting both.

@manidlou
Copy link
Collaborator

agreed!

RyanZim added a commit that referenced this issue Feb 14, 2020
@RyanZim RyanZim added this to the 9.0.0 milestone Feb 14, 2020
RyanZim added a commit that referenced this issue Feb 17, 2020
* Refactor internal getStats() util

Fixes #762

* Proper promise tests for copy()

Our hacky tests don't play well with multiple layers of
callback/promise switching

* Simplify internal checkParentPaths()

* Port improvments to sync methods
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants