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

trace scope tracking is not happy with concurrency #2266

Open
RalfJung opened this issue Jun 25, 2022 · 9 comments
Open

trace scope tracking is not happy with concurrency #2266

RalfJung opened this issue Jun 25, 2022 · 9 comments
Assignees
Labels
A-concurrency Area: affects our concurrency (multi-thread) support A-ux Area: This affects the general user experience C-bug Category: This is a bug.

Comments

@RalfJung
Copy link
Member

When running a concurrent program with MIRI_LOG=info, the nesting depth of the resulting trace is huge even when logging code that happens directly on the main thread. I think tracing might not deal well with the fact that we have multiple stacks that are independently nested? @oli-obk is there anything we can do to improve that?

@oli-obk
Copy link
Contributor

oli-obk commented Jun 27, 2022

oh 🤦 yea. We could pop all the current thread's frames and push the new thread's frames on thread switching. Could end up expensive, but only happens while you're logging, so it should be fine.

@oli-obk oli-obk self-assigned this Jun 27, 2022
@RalfJung
Copy link
Member Author

RalfJung commented Jun 27, 2022

Back then someone said the trace library supports a notion of "thread" (or so) and we could use that. What happened to that statement?^^

@oli-obk
Copy link
Contributor

oli-obk commented Jun 27, 2022

I don't think we can hijack that for our purposes, but I'll look into it.

@RalfJung RalfJung added C-bug Category: This is a bug. A-ux Area: This affects the general user experience A-concurrency Area: affects our concurrency (multi-thread) support labels Jun 29, 2022
@RalfJung
Copy link
Member Author

RalfJung commented Mar 3, 2024

So does this need a new feature in tracing-tree first? Should we file a feature request there?

@oli-obk
Copy link
Contributor

oli-obk commented Mar 3, 2024

I think this is the same issue as async tracing has

davidbarsky/tracing-tree#60

davidbarsky/tracing-tree#74

@RalfJung
Copy link
Member Author

RalfJung commented Mar 3, 2024

But those are both closed...

@RalfJung
Copy link
Member Author

RalfJung commented Mar 3, 2024

davidbarsky/tracing-tree#74 seems to indicate that we can already have multiple independent scopes, they are just not rendered properly? Currently Miri doesn't even tell tracing about the threads so there's no way it can do anything. All we do is:

        let span = info_span!("frame", "{}", instance);
        self.frame_mut().tracing_span.enter(span);

We need some way to manually set the parent span I think?

Or at least, something has to happen when we switch threads. Should we "exit" all the spans of the old thread and "enter" all the spans of the new thread? That needs spans that can be exited and entered multiple times. Also we don't want the full stack trace of leaving and entering all these spans when switching threads I think... though maybe sometimes we do.

bors added a commit that referenced this issue Mar 3, 2024
log when we change the active thread, and fix logging for concurrency

Also avoid relying on the incorrect scope exit logging produced by tracing (Cc #2266)
@ten3roberts
Copy link

Adding to this.

Yes, tracing-tree has no notion of threads. However, there is a new feature called retracing, which will re-open a span and its parents up to the common ancestor of the last outputted span during an event.

The previous behavior was to just straight up print the event when it came, ignoring if the last printed span is correct or not, the new retracing feature (off by default for now), will reprint the spans leading up to the event.

Due to this, tracing/tracing-tree does not need to know about threads, as the relationships between an events associated span (and its parents) are enough to deduce a proper tree.

There was a slight issue regarding an oversight in the combination with verbose entry and eager spans, and that is currently in motion in davidbarsky/tracing-tree#78 to be reviewed and merged.

If you want to you can try out that branch in miri, and enable retracing and see how it fares in miri. I'll be happy to help fix any issue with it if it happens to arise.

@RalfJung
Copy link
Member Author

RalfJung commented Mar 4, 2024

Due to this, tracing/tracing-tree does not need to know about threads, as the relationships between an events associated span (and its parents) are enough to deduce a proper tree.

How is the parent of a span determined? Currently I don't think Miri provides the required information to tracing, and I don't know how we should provide that information. But this was all set up by @oli-obk and I barely understand it anyway...

What I would have expected is to be able to say "this span is a child of that span" or "this span is a root, not child of anything". The APIs we currently use don't seem to offer that, but there may be other APIs I have been missing.

RalfJung pushed a commit to RalfJung/rust that referenced this issue Mar 9, 2024
log when we change the active thread, and fix logging for concurrency

Also avoid relying on the incorrect scope exit logging produced by tracing (Cc rust-lang/miri#2266)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-concurrency Area: affects our concurrency (multi-thread) support A-ux Area: This affects the general user experience C-bug Category: This is a bug.
Projects
None yet
Development

No branches or pull requests

3 participants