Join GitHub today
GitHub is home to over 36 million developers working together to host and review code, manage projects, and build software together.Sign up
cmd/compile: expand TestIntendedInlining to more packages and funcs #21851
#17566 is about improving the inlining cost model. Making changes to the compiler, especially touching the inlining heuristics, always has the risk of unintentionally making some funcs non-inlineable, potentially resulting in noticeable performance regressions.
It basically runs
However, the test is limited to the runtime. There are other small functions in the standard library that we want to be inlineable, and which we even tweak to appease the current heuristic. A recent example: https://go-review.googlesource.com/c/go/+/63332
I propose that we add a way to cover more funcs in other packages within
Suggestions of funcs/packages to add welcome. Happy to work on it if pointers on the above are given.
referenced this issue
Sep 14, 2017
Yes, that would be good. There is no need to duplicate the logic to determine whether or not a func is inlineable (and all the extras, such as reporting why it isn't so).
Then again, technically the tests are checking for different things. TestIntendedInlining checks if a func is inlineable, which is a decision made at compile-time. Your bytes test checks the binary to see if the symbol exists, so it checks if the func was inlined. Are all funcs that are inlineable always inlined? I honestly don't know the answer to that question.
If the answer is yes, then it's safe to merge with this test.
I have run out of ideas to add to the table, so I'm going to close this issue for now. Of course, feel free to still add more to it - but an umbrella issue to expand the table is no longer useful.
I believe it was @aclements who suggested that we could use benchmarks to identify what runtime functions should always be inlineable, as opposed to human guesses. That could be a way forward if someone wants to look into it.