7 ms·

os.Readdir has to be one of the stupidest, most broken interfaces I've ever seen. Reading the whole dir into an array is bad enough as it is, but they even cal

os.Readdir has to be one of the stupidest, most broken interfaces I've ever seen. Reading the whole dir into an array is bad enough as it is, but they even call stat() on every file too.

mpfundstein 6y ago
i like the function. saved me a lot of manual work already ;-) just use it when you need it and be aware of the downsides...
- boring_twenties 6y ago
  Uh, k? What does being aware of the downsides get you? Is there some alternate interface that avoids the downsides?
  - mpfundstein 6y ago
    I am just saying that the function has merit. Even if it could have been written much better. Thats all :-) It served me well. Reg downsides: Maybe don't use it on a directory with 1000 entries...? Its good to be aware of that or not?
    - boring_twenties 6y ago
      So just what the fuck are you supposed to use on a directory with 1000 entries?
      - icholy 6y ago
        You can drop down into the lower level apis https://godoc.org/golang.org/x/sys/unix https://godoc.org/golang.org/x/sys/unix
        doteka 6y ago
        Yeah, nope. There are many appropriate levels of abstraction in between “do this the dumbest way possible” and “write raw unix syscalls using a library outside of the standard distribution”.
        icholy 6y ago
        Just because something doesn't work for your use-case doesn't make it the "dumbest way possible". It just makes you sound dumb.
        doteka 6y ago
        No, this is literally the dumbest way possible to accomplish the task. It is as close to a brute force approach as you can get, I can see no way in which it could be less efficient without being malicious. And it is the only way of accomplishing this task in the standard library of a language which prides itself on performance and maintainability. Also, thanks for the ad hominem, perhaps you shouldn’t attach your self worth to a programming language and see how that works out for you.
        icholy 6y ago
        There's https://golang.org/pkg/os/#File.Readdir https://golang.org/pkg/os/#File.Readdir
        Someone 6y ago
        Which, as far as I can tell (couldn’t find opendir/closedir), has the same problem. I think they named the call after the posix one. If so, they must have known about opendir/closedir.
        icholy 6y ago
        It uses https://golang.org/pkg/syscall/#ReadDirent https://golang.org/pkg/syscall/#ReadDirent and https://golang.org/pkg/syscall/#ParseDirent https://golang.org/pkg/syscall/#ParseDirent to incrementally read them.
      - hu3 6y ago
        https://github.com/karrick/godirwalk https://github.com/karrick/godirwalk
    - doteka 6y ago
      Let’s not do participation trophies though, Pike and Thompson were the original authors of Go. Both of them should know better. It’s fine to include an inefficient version if you also have a ‘proper’ way to fall back to when performance does matter. And if performance does not matter, why are you even using Go? Funnily enough Python Ruby and Node do expose performant ways to do this in the stdlib, so...
    - deleted 6y ago
      [deleted]
  - Mathnerd314 6y ago
    This is the syscall: https://www.man7.org/linux/man-pages/man2/getdents.2.html https://www.man7.org/linux/man-pages/man2/getdents.2.html You opendir, read 100 entries, do whatever with them, read 100 more, repeat, then closedir. So it's a buffered interface similar to normal I/O. There's been proposals for doing readdir+stat in one syscall, but nothing merged yet. https://lwn.net/Articles/606995/ https://lwn.net/Articles/606995/
    - yxhuvud 6y ago
      You can also use io_uring and at least do all the stats in a single syscall (or none, depending on what flags you create the ring with, but for normal use it would be one call).
      - Mathnerd314 6y ago
        Ah yes, forgot about uring. Apparently readdir was "next on the list" at the end of last year: https://twitter.com/mathjock/status/1205996402930855937 https://twitter.com/mathjock/status/1205996402930855937 But I don't see it in the tree. I'm guessing sockets got more priority.
- Varriount 6y ago
  And what if I'm searching through a large directory? Your response doesn't address the argument being made. Creating an API where the entire set of data is returned from a data source, without an option to limit the result, is bad. It means that, if I have to read a directory with 2000 files (and this is quite possible! - think /bin, etc.) the function will have to allocate a slice with 2000 entries, and then call stat on all of them. Stat is not free, especially on certain filesystems (NFS!), nor is reading directory entries.
  - mpfundstein 6y ago
    that's why I wrote: be aware of the downside... It's a mega convenient function if you want to slurp in a few files.
    - ikiris 6y ago
      "ORM is great because it means I have all the data avaliable!"
  - sethammons 6y ago
    try millions of files :). You can run out of inodes too :)
- heavenlyblue 6y ago
  In what universe calling stat on every element of an array is “saving a lot of manual work”?
- ecnahc515 6y ago
  Yeah, it seems similar to `ioutil` functions which aren't necessarily the best approach in all cases, but are intended to make certain common tasks trivial for many cases.
wahern 6y ago
Returning large arrays is the kind of bad idiom you see in languages lacking abstract iterator constructs, generators, or lexical closures. Go isn't such a minimalist language; in fact, it's the one area where it excels, a fact often overlooked by those obsessed with generics. It's indeed perplexing why they'd add an interface that accumulates everything into a giant array by default, especially in a context that you know will cause performance problems, and used by a community with many inexperienced programmers--including ones (e.g. devops) not accustomed to thinking of these details--who will invariably fall into this trap.
- dexen 6y ago
  >why they'd add an interface that accumulates everything into a giant array by default While I can't speak for Go's authors' mindset, here's my view: One array of records is about as simple interface as possible, and it's good enough for many use cases. Contrasting with that, reading the directory with repeated readdir()'s or equivalent is fraught with hidden pitfalls. You end up being responsible for sorting, for resuming syscall in case of a signal or other interruption, for handling changes to the list of files during read... and probably other concerns above and beyond that.
  - sanderjd 6y ago
    The suggestion isn't repeated readdir calls, it is some kind of iterator, which would specify and be implemented to solve the problems you mention (which already have to be dealt with inside the array-returning readdir call, by the way).
- hombre_fatal 6y ago
  What's annoying is that filepath.Walk(), something that you'd think was the performance-minded approach, also loads all the filenames into a slice to sort it for some reason. If an API is going to arbitrarily sort filenames, at least let us pass in a comparator function so we have some control. That would actually be a decent improvement to its API.
- jeffbee 6y ago
  What is this entire subthread even about? os.(File).Readdir does not, by default, even compile. The caller has to provide a parameter of how many entries to read at a time, and reading "all of them" is an intentional choice.
hu3 6y ago
I just tested os.Readdir() on a friend's budget Windows laptop which has a consumer grade SATA SSD running on the slow cygwin over the ancient NTFS format. os.Readdir() fetched 10.000 file names into a slice in 0.2s. That's more than fine for most use cases and certainly doesn't look like "one of the stupidest, most broken interfaces I've ever seen". Specially given that it's not the only way to iterate over files of a directory in Go. There's filepath.Walk() for one.
- boring_twenties 6y ago
  filepath.Walk() isn't much better: > The files are walked in lexical order, which makes the output deterministic but means that for very large directories Walk can be inefficient. If it's fine for "most use cases," that's all well and good. Just implement the correct interface, which is fine for ALL use cases, and then implement this sugar on top of that.
  - hu3 6y ago
    They are aware: https://github.com/golang/go/issues/16399 https://github.com/golang/go/issues/16399 Seems like they postponed incorporating fastwalk into the standard library because it would break API: https://godoc.org/golang.org/x/tools/internal/fastwalk https://godoc.org/golang.org/x/tools/internal/fastwalk Perhaps until then one could use libraries that expose fastwalk: https://github.com/karrick/godirwalk https://github.com/karrick/godirwalk https://github.com/s12chung/fastwalk https://github.com/s12chung/fastwalk
- jfkebwjsbx 6y ago
  2 ms per 10 files is a huge amount of time.
  - hu3 6y ago
    You're right! I was executing time go run main.go I now built the exe and ran: real 0m0.199s user 0m0.000s sys 0m0.014s So 0.2s for 10k files on the worst possible hardware/software scenario I could find nearby. Edited my original comment. Thanks!
duskwuff 6y ago
Even when you know that you're dealing with a small number of files, and performance isn't critical? It's not a "stupid" interface if it does exactly what you need. And I think this is a pretty common use case.
- boring_twenties 6y ago
  Go already has en entire namespace devoted to making things easier for "pretty common use cases:" ioutil. That's exactly where this version of Readdir belongs. Otherwise, you'd better be 110% sure of this: > Even when you know that you're dealing with a small number of files, and performance isn't critical? Because if you turn out to be wrong down the line, your only option is now to rewrite the whole thing in a less-stupid language. Might as well just save yourself the trouble and do that from the beginning.
networkimprov 6y ago
There is a proposal to replace os.Readdir() here: https://github.com/golang/go/issues/40352 https://github.com/golang/go/issues/40352

os.Readdir has to be one of the stupidest, most broken interfaces I've ever seen. Reading the whole dir into an array is bad enough as it is, but they even call stat() on every file too.

mpfundstein 6y ago

i like the function. saved me a lot of manual work already ;-) just use it when you need it and be aware of the downsides...

boring_twenties 6y ago
Uh, k? What does being aware of the downsides get you? Is there some alternate interface that avoids the downsides?
- mpfundstein 6y ago
  I am just saying that the function has merit. Even if it could have been written much better. Thats all :-) It served me well. Reg downsides: Maybe don't use it on a directory with 1000 entries...? Its good to be aware of that or not?
  - boring_twenties 6y ago
    So just what the fuck are you supposed to use on a directory with 1000 entries?
    - icholy 6y ago
      You can drop down into the lower level apis https://godoc.org/golang.org/x/sys/unix https://godoc.org/golang.org/x/sys/unix
      - doteka 6y ago
        Yeah, nope. There are many appropriate levels of abstraction in between “do this the dumbest way possible” and “write raw unix syscalls using a library outside of the standard distribution”.
        icholy 6y ago
        Just because something doesn't work for your use-case doesn't make it the "dumbest way possible". It just makes you sound dumb.
        doteka 6y ago
        No, this is literally the dumbest way possible to accomplish the task. It is as close to a brute force approach as you can get, I can see no way in which it could be less efficient without being malicious. And it is the only way of accomplishing this task in the standard library of a language which prides itself on performance and maintainability. Also, thanks for the ad hominem, perhaps you shouldn’t attach your self worth to a programming language and see how that works out for you.
        icholy 6y ago
        There's https://golang.org/pkg/os/#File.Readdir https://golang.org/pkg/os/#File.Readdir
        Someone 6y ago
        Which, as far as I can tell (couldn’t find opendir/closedir), has the same problem. I think they named the call after the posix one. If so, they must have known about opendir/closedir.
        icholy 6y ago
        It uses https://golang.org/pkg/syscall/#ReadDirent https://golang.org/pkg/syscall/#ReadDirent and https://golang.org/pkg/syscall/#ParseDirent https://golang.org/pkg/syscall/#ParseDirent to incrementally read them.
    - hu3 6y ago
      https://github.com/karrick/godirwalk https://github.com/karrick/godirwalk
  - doteka 6y ago
    Let’s not do participation trophies though, Pike and Thompson were the original authors of Go. Both of them should know better. It’s fine to include an inefficient version if you also have a ‘proper’ way to fall back to when performance does matter. And if performance does not matter, why are you even using Go? Funnily enough Python Ruby and Node do expose performant ways to do this in the stdlib, so...
  - deleted 6y ago
    [deleted]
- Mathnerd314 6y ago
  This is the syscall: https://www.man7.org/linux/man-pages/man2/getdents.2.html https://www.man7.org/linux/man-pages/man2/getdents.2.html You opendir, read 100 entries, do whatever with them, read 100 more, repeat, then closedir. So it's a buffered interface similar to normal I/O. There's been proposals for doing readdir+stat in one syscall, but nothing merged yet. https://lwn.net/Articles/606995/ https://lwn.net/Articles/606995/
  - yxhuvud 6y ago
    You can also use io_uring and at least do all the stats in a single syscall (or none, depending on what flags you create the ring with, but for normal use it would be one call).
    - Mathnerd314 6y ago
      Ah yes, forgot about uring. Apparently readdir was "next on the list" at the end of last year: https://twitter.com/mathjock/status/1205996402930855937 https://twitter.com/mathjock/status/1205996402930855937 But I don't see it in the tree. I'm guessing sockets got more priority.
Varriount 6y ago
And what if I'm searching through a large directory? Your response doesn't address the argument being made. Creating an API where the entire set of data is returned from a data source, without an option to limit the result, is bad. It means that, if I have to read a directory with 2000 files (and this is quite possible! - think /bin, etc.) the function will have to allocate a slice with 2000 entries, and then call stat on all of them. Stat is not free, especially on certain filesystems (NFS!), nor is reading directory entries.
- mpfundstein 6y ago
  that's why I wrote: be aware of the downside... It's a mega convenient function if you want to slurp in a few files.
  - ikiris 6y ago
    "ORM is great because it means I have all the data avaliable!"
- sethammons 6y ago
  try millions of files :). You can run out of inodes too :)
heavenlyblue 6y ago
In what universe calling stat on every element of an array is “saving a lot of manual work”?
ecnahc515 6y ago
Yeah, it seems similar to `ioutil` functions which aren't necessarily the best approach in all cases, but are intended to make certain common tasks trivial for many cases.

wahern 6y ago

Returning large arrays is the kind of bad idiom you see in languages lacking abstract iterator constructs, generators, or lexical closures. Go isn't such a minimalist language; in fact, it's the one area where it excels, a fact often overlooked by those obsessed with generics. It's indeed perplexing why they'd add an interface that accumulates everything into a giant array by default, especially in a context that you know will cause performance problems, and used by a community with many inexperienced programmers--including ones (e.g. devops) not accustomed to thinking of these details--who will invariably fall into this trap.

dexen 6y ago
>why they'd add an interface that accumulates everything into a giant array by default While I can't speak for Go's authors' mindset, here's my view: One array of records is about as simple interface as possible, and it's good enough for many use cases. Contrasting with that, reading the directory with repeated readdir()'s or equivalent is fraught with hidden pitfalls. You end up being responsible for sorting, for resuming syscall in case of a signal or other interruption, for handling changes to the list of files during read... and probably other concerns above and beyond that.
- sanderjd 6y ago
  The suggestion isn't repeated readdir calls, it is some kind of iterator, which would specify and be implemented to solve the problems you mention (which already have to be dealt with inside the array-returning readdir call, by the way).
hombre_fatal 6y ago
What's annoying is that filepath.Walk(), something that you'd think was the performance-minded approach, also loads all the filenames into a slice to sort it for some reason. If an API is going to arbitrarily sort filenames, at least let us pass in a comparator function so we have some control. That would actually be a decent improvement to its API.
jeffbee 6y ago
What is this entire subthread even about? os.(File).Readdir does not, by default, even compile. The caller has to provide a parameter of how many entries to read at a time, and reading "all of them" is an intentional choice.

hu3 6y ago

I just tested os.Readdir() on a friend's budget Windows laptop which has a consumer grade SATA SSD running on the slow cygwin over the ancient NTFS format. os.Readdir() fetched 10.000 file names into a slice in 0.2s. That's more than fine for most use cases and certainly doesn't look like "one of the stupidest, most broken interfaces I've ever seen". Specially given that it's not the only way to iterate over files of a directory in Go. There's filepath.Walk() for one.

boring_twenties 6y ago
filepath.Walk() isn't much better: > The files are walked in lexical order, which makes the output deterministic but means that for very large directories Walk can be inefficient. If it's fine for "most use cases," that's all well and good. Just implement the correct interface, which is fine for ALL use cases, and then implement this sugar on top of that.
- hu3 6y ago
  They are aware: https://github.com/golang/go/issues/16399 https://github.com/golang/go/issues/16399 Seems like they postponed incorporating fastwalk into the standard library because it would break API: https://godoc.org/golang.org/x/tools/internal/fastwalk https://godoc.org/golang.org/x/tools/internal/fastwalk Perhaps until then one could use libraries that expose fastwalk: https://github.com/karrick/godirwalk https://github.com/karrick/godirwalk https://github.com/s12chung/fastwalk https://github.com/s12chung/fastwalk
jfkebwjsbx 6y ago
2 ms per 10 files is a huge amount of time.
- hu3 6y ago
  You're right! I was executing time go run main.go I now built the exe and ran: real 0m0.199s user 0m0.000s sys 0m0.014s So 0.2s for 10k files on the worst possible hardware/software scenario I could find nearby. Edited my original comment. Thanks!

duskwuff 6y ago

Even when you know that you're dealing with a small number of files, and performance isn't critical? It's not a "stupid" interface if it does exactly what you need. And I think this is a pretty common use case.

boring_twenties 6y ago
Go already has en entire namespace devoted to making things easier for "pretty common use cases:" ioutil. That's exactly where this version of Readdir belongs. Otherwise, you'd better be 110% sure of this: > Even when you know that you're dealing with a small number of files, and performance isn't critical? Because if you turn out to be wrong down the line, your only option is now to rewrite the whole thing in a less-stupid language. Might as well just save yourself the trouble and do that from the beginning.

networkimprov 6y ago

There is a proposal to replace os.Readdir() here: https://github.com/golang/go/issues/40352 https://github.com/golang/go/issues/40352