janestreet / magic-trace

magic-trace collects and displays high-resolution traces of what a process is doing
https://magic-trace.org
MIT License
4.58k stars 87 forks source link

Make gofmt look good #200

Open cgaebel opened 2 years ago

cgaebel commented 2 years ago

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

prattmic commented 2 years ago

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 commented 2 years ago

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