cmd/compile: ssa.a contains the string SetsockoptIPv6Mreq #20084
Comments
josharian
changed the title
|
To help quantify, CL 41493 contains a quick hack to remove explicit references to |
DO NOT SUBMIT This is a demo CL for golang#20084. It currently reduces the size of the ssa export data by 10%, from 76154 to 67886. Change-Id: I49e8951c5bfce63ad2b7f4fc3bfa0868c53114f9
|
This seems like another good example of a situation where using an interface to abstract enormous amounts of detail (which in turn turns out as type information) would help. Having an abstract File notion (which has come up many times before) would probably avoid pulling in these details. |
|
@griesemer agreed, although it'd also be better for the compiler to handle this more gracefully as well. I spent some time on this today but didn't make much progress. There are at least a few things this apparently inaccessible data is being used for: (1) method sets, (2) hash/eq routines, (3) sanity checks across imported packages, (4) re-typechecking of inlined method bodies. (1) and (2) could be represented more parsimoniously. (3) is nice but substantively dropping the cost of compilation is probably nicer. (4) could possibly be worked around. I wonder whether the sanity checks could be protected behind a flag, in favor of not constructing the type the second time around, and if so, how much that would save in practice. |
|
I just updated CL 41493 with some numbers: This improved compilation time for a downstream package (cmd/compile/internal/amd64) by > 3%. Over a large compilation graph, like benchjuju has, this could be significant. Moving to 1.10, though, because I don't see this happening in the next week. |
This reduces the size of the ssa export data by 10%, from 76154 to 67886. It doesn't appear that #20084, which would do this automatically, is going to be fixed soon. Do it manually for now. This speeds up compiling cmd/compile/internal/amd64 and presumably its comrades as well: name old time/op new time/op delta CompileAMD64 89.6ms ± 6% 86.7ms ± 5% -3.29% (p=0.000 n=49+47) name old user-time/op new user-time/op delta CompileAMD64 116ms ± 5% 112ms ± 5% -3.51% (p=0.000 n=45+42) name old alloc/op new alloc/op delta CompileAMD64 26.7MB ± 0% 25.8MB ± 0% -3.26% (p=0.008 n=5+5) name old allocs/op new allocs/op delta CompileAMD64 223k ± 0% 213k ± 0% -4.46% (p=0.008 n=5+5) Updates #20084 Change-Id: I49e8951c5bfce63ad2b7f4fc3bfa0868c53114f9 Reviewed-on: https://go-review.googlesource.com/41493 Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com> TryBot-Result: Gobot Gobot <gobot@golang.org> Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
|
CL https://golang.org/cl/41493 mentions this issue. |
This reduces the size of the ssa export data by 10%, from 76154 to 67886. It doesn't appear that golang#20084, which would do this automatically, is going to be fixed soon. Do it manually for now. This speeds up compiling cmd/compile/internal/amd64 and presumably its comrades as well: name old time/op new time/op delta CompileAMD64 89.6ms ± 6% 86.7ms ± 5% -3.29% (p=0.000 n=49+47) name old user-time/op new user-time/op delta CompileAMD64 116ms ± 5% 112ms ± 5% -3.51% (p=0.000 n=45+42) name old alloc/op new alloc/op delta CompileAMD64 26.7MB ± 0% 25.8MB ± 0% -3.26% (p=0.008 n=5+5) name old allocs/op new allocs/op delta CompileAMD64 223k ± 0% 213k ± 0% -4.46% (p=0.008 n=5+5) Updates golang#20084 Change-Id: I49e8951c5bfce63ad2b7f4fc3bfa0868c53114f9
|
The string "SetsockoptIPv6Mreq" is not there anymore with tip (2f588ff). Is there anything else expected to be done in this issue? @josharian |
This is a bit surprising. How did it get there?
There are a few places in the ssa package where we have types that refer to
*os.File, likemap[string]*os.File. When exporting, we re-export*os.File, which isstruct { *os.file }.os.fileis (on my machine)struct { os.pfd poll.FD; os.name string; os.dirinfo *os.dirInfo; os.nonblock bool }.poll.FDisstruct { poll.fdmu poll.fdMutex; Sysfd int; poll.pd poll.pollDesc; poll.iovecs *[]syscall.Iovec; IsStream bool; ZeroReadIsEOF bool }.And so on all the way down to
SetsockoptIPv6Mreq, leaving a trail of types in our wake.I've been trying to think of how the compiler might possibly need these types in a package that imports only
cmd/compile/internal/ssa. It doesn't need them directly for typechecking, becauseos.fileis not exported, and thus unreachable via our code. The struct size/shape/alignment doesn't matter, because it is only reference is via a pointer (*os.file); same reasoning for GC maps.I'd love enlightenment.
If they're not needed, we should figure out how to get rid of them. Maybe part of the problem in #20070 is that our object files have lots of stuff in them that we don't even need in the first place?
@griesemer @mdempsky
The text was updated successfully, but these errors were encountered: