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: optimise away deferred calls to empty functions #26534
Comments
FWIW, I went further and tested it with and without closures, and also with the go statement. The results are: assert(x == 0) // Optimised.
func() {}() // Optimised.
func() { assert(x == 1) }() // Optimised.
defer assert(x == 0) // Not optimised.
defer func() {}() // Not optimised.
defer func() { assert(x == 1) }() // Not optimised.
go assert(x == 0) // Not optimised.
go func() {}() // Not optimised.
go func() { assert(x == 1) }() // Not optimised. |
The optimized cases you see are only optimized because of inlining. If you change it to: //go:noinline
func assert(cond bool) { /* NOOP in this build tag. */ } then you'll see all present. Recognizing and eliminating Is it safe to infer that this happens to you with real code, where a function is empty due to build tags, and that the defer calls are a performance problem? |
Good catch with the inlining, and thanks for the input! There is no real code at the moment, I'm just tinkering with the concept of a package for contact programming in Go. The interface is still in flux, but the main idea is that all methods are NOOPs, unless a build tag is specified. Preconditions are checked immediately, and postconditions are checked in a deferred function. Ideally, I would like to be able to write something like this in the package's README:
But depending on the interface I will end up with, this might be a hard promise to keep. |
I've also been pondering contract-based programming in Go, so I hope that you will write up the results of your experiment and share it. The fact that this issue isn't motivated by existing code with performance issues makes it a bit lower priority, realistically, but it is (I believe) a simple enough fix that someone will decide to tackle it anyway. |
Please note that the language specification does not guarantee anything of that. It may be true for a particular version of a particular Go compiler, at most. |
@josharian Seems like something I could do using ast, but I would like direction as to which file to look for the optimization functions in. If it is not taken I'd like to tackle it with a bit of guidance (: |
I am also interested in working on this issue so we can join the efforts, I was looking at the compile documentation and I think that the optimization should happen either in Generic SSA or Type-checking and AST transformations phase. I am more inclined to do this in SSA phase as there is stated following:
It seems like the best place to put this optimizations is cmd/compile/internal/ssa/deadcode.go as I consider these functions as a deadcode. I will investigate more once I catch up on with SSA and receive some sort of ack or anything :) |
I also read a bit about SSA today, tomorrow I'll dive into the code and understand exactly how we should do it (: @rkuska I sent you an email with further discussion |
If you see a clean way to do this in SSA world, that'd be great. I had had in mind catching this case in cmd/compile/internal/gc/walk.go, func walkstmt, case ODEFER / OPROC. (OPROC should be renamed to OGO; no one has done that yet.) If you get a CL together, please be sure to add fairly extensive tests, including inlineable and non-inlineable (via Sorry for the slow response, I have very little computer time at the moment. |
I found a good place to do it in SSA deadcode.go, when we check for live code in functions. That way we eliminate all calls to the function. Should I push the OPROC -> OGO change with this CL or create a new issue? |
@josharian Do we prefer to remove the function if it's a no-op function and let the SSA do it's own thing by removing the calls, or should we remove the calls to no-op functions but leave the functions as is? |
Anyone familiar with the compiler has some time to answer a few questions? |
Please feel free to reach out to golang-dev mailing list. |
@agnivade I asked my question there but no one answered, so I thought I better try here |
I'll do my best to answer questions, but I have very limited computer time, so expect significant delays. Hopefully others will also find time to chip in. Ideally we can try to keep the conversation in one place, though, to make it easier to reply. For now I'll reply in each place to the questions asked there.
This is very low priority. The name has been bad for years now. No need to create an issue for it. Feel free to just send a CL, but if you do so, (a) please prepare the CL using gorename, (b) please don't mix it with any other changes, to make it easier to review.
I don't really understand this question, I'm afraid. Another thing to test, by the way: type T int
func (t T) f() {}
func main() {
var t *T
t.f()
} This will panic-- |
I have something working, I'll push it later today, it doesn't interfere with methods so no problem with the case of nil object. |
Change https://golang.org/cl/128035 mentions this issue: |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
What operating system and processor architecture are you using (
go env
)?What did you do?
https://play.golang.org/p/TbjFF3fe8Wu
What did you expect to see?
Lines 12, 13, and 14 skipped by the compiler.
What did you see instead?
Line 12 is skipped, but defers on 13 and 14 are still there. The Go compiler leaves those in, despite the fact that they are NOOPs.
This makes it hard to create assertions (in this case, postconditions) that don't hurt the performance of production builds using build tags.
The text was updated successfully, but these errors were encountered: