Skip to content
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: optimize print call sequences #48828

Closed
josharian opened this issue Oct 6, 2021 · 4 comments
Closed

cmd/compile: optimize print call sequences #48828

josharian opened this issue Oct 6, 2021 · 4 comments
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance Suggested Issues that may be good for new contributors looking for work to do.
Milestone

Comments

@josharian
Copy link
Contributor

The runtime is full of debugging calls of the form println("a=", hex(a), "b=", hex(b)), followed by a throw. These are very useful when something goes wrong, but the generated code for them is very verbose. It can easily account for half of the generated code in hot runtime routines. It is placed at the end of the function, but for icache purposes, it'd be better for it to be outlined and placed far away.

Given the poor share the inliner is in, I'm going to assume that we're not going to add an outliner (even one manually triggered) any time soon.

As an easier fix, I did some work a year ago on this, adding a bunch of helper print routines, like printonestring, which does printlock; printstring; printunlock, all in a single call. And there are probably other helpful combinations I didn't add, like printstringhex, which does printstring; printhex. (An analysis of call ngrams would be useful.) It might also be useful to have a printstringptr that accepts a pointer to a string to print, instead of the string itself, for use particularly with constant strings. And maybe there are other inventive solutions that can shrink call sites further.

The work I did is at josharian@9d18ed5, with preparatory commits josharian@18a0eff and josharian@ce3a481.

It no longer applies cleanly due to substantive compiler refactoring in the intervening year, but perhaps someone is interested in re-creating those patches at tip. And maybe improving them.

Because there's some existing code written to work from, this might be a good first issue for someone new to the compiler.

@josharian josharian added Suggested Issues that may be good for new contributors looking for work to do. Performance labels Oct 6, 2021
@rsc
Copy link
Contributor

rsc commented Oct 6, 2021

How big are the icache lines? 64 bytes?
It seems hard to believe that if the prints are at the end of the function,
that they are taking up any space in hot lines.

@mknyszek mknyszek added this to the Backlog milestone Oct 7, 2021
@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Oct 7, 2021
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022
@RyanMuzzey
Copy link

Is this still an issue available to new contributors?

@RyanMuzzey
Copy link

It looks like this was addressed in this change which added lock before and unlock after the print functions are called. Is there still a need for further changes here, or can this issue be closed?

@josharian
Copy link
Contributor Author

That change was just code re-arrangement. (It even comments as much in the commit message.) Given Russ's comments above, though, this is probably not a good place to spend time. I'll close it. Thanks for poking at it, @RyanMuzzey .

@golang golang locked and limited conversation to collaborators Jan 8, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Performance Suggested Issues that may be good for new contributors looking for work to do.
Projects
None yet
Development

No branches or pull requests

5 participants