RFC: Change 'readdir' to return an iterator.

[malmaud]

cf #27393

[StefanKarpinski]

Aside from iterator name bikeshedding, looks good to me.

[StefanKarpinski]

This seems unnecessarily abbreviated. How about ReadDirIterator?

[malmaud]

Sure

[vtjnash]

It would be simpler (and faster and more memory efficient) to implement this as:

lazy_readdir(name) = (path for path in readdir(name))

Just my 2¢

[malmaud]

Wait, how is that lazy? It would still be loading all the paths into memory at once, wouldnt it?

…

On Wed, Jun 6, 2018 at 10:28 AM Jameson Nash ***@***.***> wrote: It would be simpler (and faster and more memory efficient) to implement this as: lazy_readdir(name) = (path for path in readdir(name)) Just my 2¢ — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#27450 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA8SvaFfxgBbBvhBo73ZgMLKWw3GXGspks5t5-cWgaJpZM4Ub10q> .

[vtjnash]

They are already all loaded into memory, this PR would just make accessing them slower.

Support for the lazy version was proposed a few years ago, so it might land soon(ish) in libuv master: libuv/libuv#416

[StefanKarpinski]

I guess the question for us at this point is: do we want to expose an iterator interface to readdir even though the current libuv implementation is always eager in anticipation of libuv soon having a truly lazy implementation for which an iterator interface can be advantageous. I would say yes. The choice of implementation doesn't matter much and @malmaud's one here has the advantage of possibly not needing to change when libuv changes theirs.

[malmaud]

I agree with that. Ill also run a quick benchmark just in case there really is a non trivial performance hit.

…

On Wed, Jun 6, 2018 at 10:41 AM Stefan Karpinski ***@***.***> wrote: I guess the question for us at this point is: do we want to expose an iterator interface to readdir even though the current libuv implementation is always eager in anticipation of libuv soon having a truly lazy implementation for which an iterator interface can be advantageous. I would say yes. The choice of implementation doesn't matter much and @malmaud <https://github.com/malmaud>'s one here has the advantage of possibly not needing to change when libuv changes theirs. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#27450 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA8Sva49xA3SNBWMRokfDYdOzG88T3Rkks5t5-oggaJpZM4Ub10q> .

[StefanKarpinski]

It's hitting the file system. Speed doesn't matter.

[JeffBezanson]

I don't see why an iterator is better. Arrays support more operations, e.g. pmap. Directories hardly ever (never?) have so many entries that memory use would be prohibitive.

[StefanKarpinski]

The fact that it's an issue for libuv would seem to indicate that there are cases where it matters. Otherwise they would presumably not be bothering to change this.

[StefanKarpinski]

Relevant motivational issues:

• A proposal to add fs.scandir method to FS module nodejs/node#15699
• fs.readdir callback is an array which is inefficient to process for large directories nodejs/node-v0.x-archive#388

[malmaud]

Yes, I agree that the fact that most other libraries already do this lazily or have open issues from people running into trouble from it not being lazy should tell us something.

I think there's no practical limit in Linux anymore on how many files can be in a directory. Thus someone, somewhere, will run into performance issues that we can never remedy once we've committed to an API where readdir returns a fully reified list.

We could potentially just add another function like Jameson's lazy_readdir or whatever, if people want to keep readdir as it is.

[JeffBezanson]

Ok, I'm convinced, we can make it an iterator.

[malmaud]

Big mistake.

[StefanKarpinski]

Note that to address the "double file system call" issue in nodejs/node#15699 we would need to return some kind of FileNode object that acts like a file path but caches stat information (which it already has from the readdir operation). It's unclear to me how relevant the "if we list large directories the whole process hangs" issue in nodejs/node#15699 is to Julia—after all, we can make readdir internally non-blocking and let other tasks do stuff while readdir collects directory names. Blocking isn't the huge problem in Julia that it is in JavaScript.

[StefanKarpinski]

OTOH, I think it's just good practice to have streaming APIs to things that can be done incrementally. That will guide people naturally towards writing more scalable code for this sort of thing.

[StefanKarpinski]

Bump, it's now or never. This is unlikely to break too terribly many usages since the most common usage is simply to iterate through the output of readdir...

[vtjnash]

It's still not clear to me how often you would need to use this function on directories with more than tens of millions of entries, such that you would want to slightly increase the cost and complexity of using readdir.

[StefanKarpinski]

If that's the call, ok. If it becomes an issue in the future, we could have readdir(dir, lazy=true) or something like it in the future.

[malmaud]

Alright, shall we just do Stefan's suggestion then (presumably a type-stable variant) at some point then?