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: postpone argument conversions after inlining #20087

Open
rasky opened this Issue Apr 23, 2017 · 4 comments

Comments

Projects
None yet
5 participants
@rasky
Member

rasky commented Apr 23, 2017

Look at this code:

package main

import (
	"fmt"
)

type Log struct {
	Enabled bool
}

func (l *Log) Print(args ...interface{}) {
	if l.Enabled {
		fmt.Println(args...)
	}
}

func main() {
	var x int = 1000
	l := Log{Enabled: false}
	l.Print("data", x)
}

If the code is compiled with -gcflags=-l=4, Log.Print() is inlined into main(). Unfortunately, this does not get rid of the expensive argument conversions to empty interfaces.

I know some work has been merged to speed up conversions of common data to empty interfaces, and that's great for when you are actually logging; but if logging is disabled, you can't fully get rid of the overhead.

I have a real high-performance program where I selectively enable logging for debugging reasons. Unfortunately, even with logging disabled, the overhead is measurable so I'm forced to actually comment out logging lines.

If the above pattern was optimized correctly, I could reword the API of my high-perf logging library to make sure such an optimization triggers for me.

@rasky rasky changed the title from cmd/compile: postpone argument conversions after inling to cmd/compile: postpone argument conversions after inlining Apr 23, 2017

@rasky

This comment has been minimized.

Member

rasky commented Apr 23, 2017

(the issue title sucks, I can't come up with a proper title, feel free to rename)

@dr2chase

This comment has been minimized.

Contributor

dr2chase commented Apr 23, 2017

If we could more aggressively sink code within the inlined "then" arm of the if statement, that would do most of what you want, right?

But wow, yuck, it's this self-referential mem-touching mess that SSA is not going to easily recognize is side-effect-free.

@cherrymui

This comment has been minimized.

Contributor

cherrymui commented Apr 23, 2017

There are actually a few things going on:

  1. The Print method has a pointer receiver, so it makes the compiler think l in main is address-taken, and therefore not SSA'd. As a consequence false is stored and the loaded later, not propagated as a constant false (because there are calls in between). The call to fmt.Println is not eliminated, neither are the arguments.

    • After inlining, l isn't necessarily address-taken though.
  2. (Even l is not address-taken) Variadic call uses array for the arguments. Array is not SSA'd. Arguments are stored to the array. So it doesn't know the arguments are actually not needed.

    • The compiler probably should think store to locals dead if it is not read by anything (calls are considered as read).
    • Even so, the call to convT2E64 is probably still not eliminated.
@rasky

This comment has been minimized.

Member

rasky commented Apr 23, 2017

If we could more aggressively sink code within the inlined "then" arm of the if statement, that would do most of what you want, right?

Yes, sounds like that would work.

@cherrymui: looks like your analysis focused on seeing why the code doesn't get turned into a big nop because of the constant false. This is interesting and could be a different bug, but it's not useful to me: in my real-world case, that boolean flag is not constant. What I'm interested into is how much useless code is being executed even if the variable is false (at runtime).

@ianlancetaylor ianlancetaylor added this to the Unplanned milestone Apr 13, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment