Sitelet https://web.archive.org/web/20211229194356/https://github.com/nodejs/node/pull/41351
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

fs: use async directory processing in cp() #41351

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

@cjihrig
Copy link
Contributor

@cjihrig cjihrig commented Dec 29, 2021

The readdir() functions do not scale well, which is why opendir(), etc. were introduced. This is exacerbated in the current cp() implementation, which calls readdir() recursively.

This commit updates cp() to use the opendir() style iteration.

The readdir() functions do not scale well, which is why
opendir(), etc. were introduced. This is exacerbated in the
current cp() implementation, which calls readdir() recursively.

This commit updates cp() to use the opendir() style iteration.
@bnb
Copy link
Member

@bnb bnb commented Dec 29, 2021

Would love to see perf metrics on this if you've got any handy 👀

lpinca
lpinca approved these changes Dec 29, 2021
bnb
bnb approved these changes Dec 29, 2021
Copy link
Member

@mcollina mcollina left a comment

lgtm

for await (const dirent of dir) {
const { name } = dirent;
Copy link
Member

@lpinca lpinca Dec 29, 2021

Choose a reason for hiding this comment

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

Suggested change
for await (const dirent of dir) {
const { name } = dirent;
for await (const { name } of dir) {

Just a nit to save a line. Feel free to ignore.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

6 participants