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

Make gofmt look good #200

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cgaebel
Copy link
Contributor

@cgaebel cgaebel commented Apr 27, 2022

This is actually two separate hacks.

Hack 1:

gogo is either used to switch to something in the current stack or
an entirely separate stack.

If we're switching to an existing stack, assume that we're resuming
execution at the lowest stack frame with the same symbol as what gogo
jumped into.

If there's no existing stack frame that matches the gogo target,
assume we're switching to an entirely separate stack.

One place where this strategy breaks down is that it doesn't work
well with recursive functions that switch between goroutines. I
suspect that's not a very common pattern in non-adversaial go code,
and hope this will work well for most people.

Testing: There was already a test around this case, I'm trying out
this new strategy to cover more bases than just what's in the demo
script. Turns out real go code can jump into more than two
functions.

Hack 2:

A goroutine that returns, when there are no other goroutines,
returns into a function called "runtime.goexit.abi0".

When magic-trace sees a return into that symbol, it ends the
current thread and pretends there's a call to goexit. The goal
here is to end the outstanding stack frames and not infer goexit
as starting at the last thread end.

Testing: I added a perf test around the complete golang exit
sequence.

cc @prattmic to take a look at this gofmt trace and point
out any remaining spots where you think magic-trace is confused.

Refs #100

@cgaebel cgaebel requested a review from Xyene April 27, 2022 00:36
This is actually two separate hacks.

Hack 1:

gogo is either used to switch to something in the current stack or
an entirely separate stack.

If we're switching to an existing stack, assume that we're resuming
execution at the lowest stack frame with the same symbol as what gogo
jumped into.

If there's no existing stack frame that matches the gogo target,
assume we're switching to an entirely separate stack.

One place where this strategy breaks down is that it doesn't work
well with recursive functions that switch between goroutines. I
suspect that's not a very common pattern in non-adversaial go code,
and hope this will work well for most people.

Testing: There was already a test around this case, I'm trying out
this new strategy to cover more bases than just what's in the demo
script. Turns out real go code can jump into more than two
functions.

Hack 2:

A goroutine that returns, when there are no other goroutines,
returns into a function called "runtime.goexit.abi0".

When magic-trace sees a return into that symbol, it ends the
current thread and pretends there's a call to goexit. The goal
here is to end the outstanding stack frames and not infer goexit
as starting at the last thread end.

Testing: I added a perf test around the complete golang exit
sequence.
@prattmic
Copy link

This trace looks quite good. It doesn't have many preemptions to look at. I'll probably need to look at a different program, though I don't have time at the moment. But I think this is good for now.

@cgaebel
Copy link
Contributor Author

cgaebel commented May 5, 2022

Here's another trace, this time with more preemptions. It still doesn't look quite right, I'm probably still missing something.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants