feat(copyFiles): improve with size filter #93
Conversation
| console.log(`Copying file ${inputPath} to ${fileDestinationPath}`); | ||
| try { | ||
| const destFileStats = FileSystem.getStatistics(fileDestinationPath); | ||
| if (destFileStats.size === pathStats.size) { |
There was a problem hiding this comment.
Hmm I wonder if there is some more (cheap) scoping conditions we could use here, like one of the timestamp members just to make sure changes that result in the same file size are caught here.
Ideally we'd use something like a file hash but that's probably prohibitively expensive a lot of the time.
There was a problem hiding this comment.
Agreed. The FileSystem.copyFiles API has an option for preserveTimestamps so that the destination aligns with the source location. That should make it so assuming you are caching builds locally, it'll only copy if the destination is older. https://github.com/microsoft/rushstack/blob/dac8e5ca3f474b6a2169a089f1b7e0e8a006237c/libraries/node-core-library/src/FileSystem.ts#L1156
There was a problem hiding this comment.
hmm, I think I can make an update that:
If srcStats.size !== destStats.size -> copy.
Else if |srcStats.mtimeMs - destStats.mtimeMs| > MTIME_TOLERANCE_MS -> copy.
Else -> skip copy.
There was a problem hiding this comment.
I have just committed the latest change!
|
I will queue this up for some team discussions next week @Keyyard, thanks a ton for the PR :) |
|
hey, i know this is kinda off topic, but i made a npm package to help developers get started with add-on creations within a minute. I wonder if i can show you here? I have made a website for it as well. Let me just attach it here just in case, this is a big pleasure of mine to having you seen it! |
this PR changed how copyFiles works by adding a filter for destination file size checking, by this when we compile it will save much more time of copying the same things again.