-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Perf work #5740
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 work #5740
Conversation
async function isSameBinaryInode(a: PortablePath, b: PortablePath) { | ||
const [aStat, bStat] = await Promise.all([ | ||
xfs.statPromise(a), | ||
xfs.statPromise(b), | ||
]); | ||
|
||
return aStat.dev === bStat.dev && aStat.ino === bStat.ino; | ||
} | ||
|
||
const isSameBinary = process.platform === `win32` | ||
? isSameBinaryContent | ||
: isSameBinaryInode; |
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.
This wont catch cases where the file content is the same but the inode isn't, for example with Corepack and yarnPath
the inode (most likely) wont be the same but the content probably is.
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.
Having exactly the same version in Corepack and yarnPath (considering that V4 use Corepack without yarnPath when possible) seems in theory unlikely enough that it's tolerable
Perhaps a problem though is that existing projects will have a hard time noticing that they should remove yarnPath
from their configuration and fully migrate to Corepack... 🤔
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.
I'll make a separate PR to add a warning when yarnPath
is detected when it could be Corepack
Co-authored-by: Kristoffer K. <[email protected]>
What's the problem this PR addresses?
This diff implements a couple of small performance improvements.
How did you fix it?
nodeUtils.builtinModules
(usemodule.builtinModules
)ppath
functions on non-win32 platformstoFilename
(just cast toFilename
if needed)yarnPath
same-file check now uses inodes on non-win32 platformsbefore
after
Checklist