-
Notifications
You must be signed in to change notification settings - Fork 17.9k
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
cmd/compile: large return value can take very long time to compile #14032
Comments
Not only it takes time to compile, it probably won't
run.
Per the Go ABI, it will be returning 1e10 bytes on
the stack. And that's almost 10GB. Not to mention
that fn will have to clear it on entry.
Given that even it compiles, it probably won't run,
I don't think this issue matters.
|
You can reduce it to 1e9 bytes, which will compile in ~30 seconds and run just fine. However, it only runs fine if using
I agree that this issue doesn't matter by itself. Nobody will/should write code like that. But I do think it is uncovering real issues that may matter in other situations. Why does it take so long to compile (is that expected or not)? Should the |
Taking long to compile is a bug. |
The compiler is stuck in a O(N) loop on array bounds to generate
pointer bitmaps. See
http://tip.golang.org/src/cmd/compile/internal/gc/plive.go#L945
we can optimize for this trivial case, but if I really don't think this
matters. The real problem is that the stack pointer maps are
not encoded, so it's O(N) bits for array [N]. Even if we fix the
compiler to not looping over array bound (by replicating the
pointer map for one array entry), the underlying problem is
still there: GC still need to traverse the huge pointer map, and
we still need to emit this huge pointer map to binary.
Without fixing this problem, making compiler compile this edge
case fast will just make it easier for Go to produce 1GB binaries
which will then trigger other problems in the toolchain (we can't
handle static data bigger than 2GB in the linker.)
|
I suggest that we change the ABI so that objects above a certain size are returned by invisible reference. The function will allocate them on the heap and return a pointer. The caller will use the pointer instead of storing the value on the stack, as it already does for large local variables (see MaxStackVarSize in cmd/compile/internal/gc). |
I'm going back and forth between milestone Go1.9 and milestone Unplanned. I feel like the compiler used to reject this kind of thing out of hand as the stack frame being too large. That seems fine to me. Not sure we should bother to make it "work". |
As a data point from my end: The code isn't from an actual use case, but rather came to exist while trying to reproduce the issue in the linked bug. I don't see a practical reason for going out of your way to make it work and failing cleanly seems fine. My only concern would be how it relates to the spec and whether it'd be a compiler limitation or not? |
With tip (638ebb0), this now compiles and executes in a moderate amount of time:
Seems good enough to me; closing. |
The following piece of code takes several minutes to compile (I gave up after a while):
Possibly but not necessarily related to #14028.
The text was updated successfully, but these errors were encountered: