I'm putting this in the 1.18 milestone because it's a regression introduced in this devel cycle.

cc @mvdan @bcmills

Maybe related to https://go-review.googlesource.com/c/go/+/317975/

CL317975 using file size as weight to control parallalism.
If go files are small enough which will trigger ulimit.

Change https://go.dev/cl/385515 mentions this issue: cmd/gofmt: limit open files to 100 simultaneously

@dim13 out of curiosity, how did you end up with a file descriptor limit of just 256? The Linux kernel has had a default of 1024 for a long time, and Systemd also defaults to a soft limit of 1024. In both instances, the hard limit is significantly higher, so a user shouldn't need root to increase the soft limit a bit.

I agree that the current gofmt logic in master may open thousands of files at once in some edge cases, so we should probably tweak that. I'm not sure that we should specifically aim to support open file limits as low as 256, though. Do we test the rest of Go with such limits, such as go build or go test? My hunch is that gofmt would not be the only command to run into issues :)

@mvdan macOS defaults to 256.

$ ulimit -a
core file size              (blocks, -c) 0
data seg size               (kbytes, -d) unlimited
file size                   (blocks, -f) unlimited
max locked memory           (kbytes, -l) unlimited
max memory size             (kbytes, -m) unlimited
open files                          (-n) 256
pipe size                (512 bytes, -p) 1
stack size                  (kbytes, -s) 8176
cpu time                   (seconds, -t) unlimited
max user processes                  (-u) 1333
virtual memory              (kbytes, -v) unlimited

Ah, thank you @ericlagergren, I hadn't noticed the original user was on Mac. So then that answers my question; we have CI machines on Mac, so that must already be tested.

@mvdan 256 is default value on macOS; and for example on OpenBSD default value is 512.

Turns out that the biggest cause here is that gofmt in master now leaks open files; I hadn't noticed that as my ulimit -n is high enough and I only tested with gofmt -l $GOROOT/src, which doesn't have enough files to run into the limit. But the parallelism will also need tweaking, as even with the leak fixed up, I still ran into issues with ulimit -n 256 in the same directory.

I'm now reviewing @mengzhuo's CL.

@mvdan couple things (on mobile so I can’t comment on the CL):

1. you can determine the max # of open files dynamically based on rlimit
2. You could wait and retry when you see whatever errno value is returned for “too many open files”

both of those avoid hardcoding 100 or 240 open files.

Both of those are indeed a possibility, but we are very late into the freeze and 1.18 is already significantly delayed, so I think we need to go with a simpler approach that's less likely to cause further delays.

Change https://go.dev/cl/385656 mentions this issue: cmd/gofmt: limit to 100 concurrent file descriptors for file reads

I've fixed this for 1.18 with a hard-coded limit of 200. If someone wants to follow up with a change to use syscall.Getrlimit, I'd be happy to review that for 1.19.

I grabbed some benchmark numbers as I was curious if the 200 hard limit would cause a performance regression. As a reminder, this is 1.17 versus what we had in master before Bryan's fix:

name         old time/op         new time/op         delta
GofmtSrcCmd          4.13s ± 6%          0.53s ± 5%   -87.08%  (p=0.000 n=12+11)

name         old user-time/op    new user-time/op    delta
GofmtSrcCmd          4.88s ± 7%          6.54s ±10%   +33.87%  (p=0.000 n=12+12)

name         old sys-time/op     new sys-time/op     delta
GofmtSrcCmd          289ms ±25%          243ms ±32%   -15.76%  (p=0.007 n=12+12)

name         old peak-RSS-bytes  new peak-RSS-bytes  delta
GofmtSrcCmd         37.8MB ± 5%        280.3MB ±19%  +641.11%  (p=0.000 n=12+12)

And this is the effect that Bryan's fix had on master:

name         old time/op         new time/op         delta
GofmtSrcCmd          534ms ± 5%          537ms ± 9%    ~     (p=0.487 n=11+12)

name         old user-time/op    new user-time/op    delta
GofmtSrcCmd          6.54s ±10%          6.67s ± 9%    ~     (p=0.060 n=12+12)

name         old sys-time/op     new sys-time/op     delta
GofmtSrcCmd          243ms ±32%          246ms ±25%    ~     (p=0.932 n=12+12)

name         old peak-RSS-bytes  new peak-RSS-bytes  delta
GofmtSrcCmd          280MB ±19%          308MB ± 9%  +9.78%  (p=0.005 n=12+12)

This is on a Ryzen 5850u with 8 physical cores, 32GiB of RAM, and a soft limit of 32k file descriptors, all on Linux 5.16.9. If a hard limit of 200 open files doesn't cause a slow-down on this machine, I imagine it will be plenty for practically all other machines. I'd even go as far as saying that using getrlimit shouldn't be worth the added complexity, unless we can prove that it actually helps in some cases - e.g. on machines with many more CPU cores.

I forgot to mention - those numbers were obtained via benchcmd -n 12 GofmtSrcCmd gofmt -l /usr/lib/go/src/cmd, where /usr/lib/go was always Go 1.17.x, but the gofmt binary was changed to each one of the three versions. I used GOROOT/src/cmd as it's decently sized and has no testdata Go files that get in the way.

@mvdan is benchcmd https://pkg.go.dev/github.com/aclements/go-misc/benchcmd?

If a hard limit of 200 open files doesn't cause a slow-down on this machine, I imagine it will be plenty for practically all other machines.

I could see it causing a slowdown when formatting a large repo hosted on a very slow disk (such as an NFS mount).

That said, if the disk is slow then everything will be slow, so it's not clear to me why we would need to optimize gofmt in particular for that case.

@ericlagergren yep!

I see another problem: 200 may be fine for now, but you never know, how many descriptors are already used. If benchmark doesn't fall into abyss, I would prefer more conservative value of maybe 128.

The limit is per-process - I'd be surprised if initializing a main Go program managed to open 56 file descriptors and keep them open. I can imagine a few, like stdin/stdout/stderr, but certainly nothing close to get from 200 to 256.

Was under wrong assumption, it were per session. But you're right, it's per process.

Thinking outloud: I wonder if #46279 would allow gofmt to assume it can use a higher limit of files. Does anyone know what darwin's default hard limit for the number of open files is, given that the default soft limit seems to be 256?