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

perf: convert chmod calls to mode arguments #3254

Merged
merged 3 commits into from
Aug 12, 2021
Merged

perf: convert chmod calls to mode arguments #3254

merged 3 commits into from
Aug 12, 2021

Conversation

paul-soporan
Copy link
Member

What's the problem this PR addresses?

We use a lot of chmod calls where writeFile (/changeFile) or mkdir with a mode argument could be used.

From my testing, a single writeFileSync(..., {mode}) call is 21.05% faster than 2 calls (a writeFileSync(...) and a chmodSync(...)):

  • 1 call - 0.15ms:
fs.writeFileSync(`a`, {mode: 0o777});
  • 2 calls - 0.19ms:
fs.writeFileSync(`a`, ``);
fs.chmodSync(`a`, 0o777);

How did you fix it?

Used the mode argument wherever possible.

Checklist

  • I have set the packages that need to be released for my changes to be effective.
  • I will check that all automated PR checks pass before the PR gets reviewed.

@paul-soporan paul-soporan requested a review from arcanis as a code owner August 9, 2021 17:22
Copy link
Member

@merceyz merceyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I looked into this a while ago but noticed ZipFS didn't implement it so I left it for another time, seems that time has come

writeFileSync(p: FSPath<PortablePath>, content: string | Buffer | ArrayBuffer | DataView, opts?: WriteFileOptions) {
const {encoding, index, resolvedP} = this.prepareWriteFile(p, opts);
if (index !== undefined && typeof opts === `object` && opts.flag && opts.flag.includes(`a`))
content = Buffer.concat([this.getFileSource(index), Buffer.from(content as any)]);
if (encoding !== null)
content = content.toString(encoding);
const newIndex = this.setFileSource(resolvedP, content);
if (newIndex !== index) {
this.registerEntry(resolvedP, newIndex);
}
}

@paul-soporan paul-soporan requested a review from merceyz August 9, 2021 17:55
@arcanis arcanis merged commit f842a6d into master Aug 12, 2021
@arcanis arcanis deleted the paul/perf/chmod branch August 12, 2021 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants