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

Differential output only calculates diff correctly for leaves (most specific frames) #295

Open
itamarst opened this issue May 31, 2023 · 4 comments

Comments

@itamarst
Copy link
Contributor

Consider a diff of these two (cargo run --bin inferno-diff-folded old1.prof new2.prof | cargo run --bin inferno-flamegraph -- - > out.svg):

parent;first_child 10
parent;second_child 10
parent2 30

and

parent;first_child 30
parent;second_child 30
parent2 30

The resulting flamegraph shows parent as unchanged, even though it's actually gotten larger:
Screenshot from 2023-05-31 14-45-11

@jonhoo
Copy link
Owner

jonhoo commented Aug 6, 2023

That's... bizarre. Looking at the code some more, I think this isn't the rendering but the actual diff collapse computation. Specifically, I wonder if it's related to this comment:

// For some reason the Perl version does a `+=` for `delta`, but I can't figure out why.
// See https://github.com/brendangregg/FlameGraph/blob/1b1c6deede9c33c5134c920bdb7a44cc5528e9a7/flamegraph.pl#L588
delta,

@jasonrhansen
Copy link
Collaborator

FWIW the current implementation matches the output of FlameGraph, and from what I can tell it's the intended behavior. See the part boxed in red below from differential-flame-graphs.

image

@jonhoo
Copy link
Owner

jonhoo commented Aug 25, 2023

I'm torn here because the current output is also pretty hard to inuit the meaning of. Maybe the trick here is going to be mainly around how we present the information in the hover popup on frames that have children. I feel like with some labels for which thing got how much slower, it might actually help a lot. I take Brendan's point that this choice is probably the one that gives more useful data, even if it is not the most intuitive one.

@itamarst
Copy link
Contributor Author

itamarst commented Sep 7, 2023

I am not sure I agree with the argument, it's useful to see which general areas had differences. On the other hand I'm wondering if differences in general are even useful for my case, since I'm including line numbers and that means minor changes to code can make a whole file non-comparable. So I'm not going to push on this thread until I get back to thinking about that...

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

No branches or pull requests

3 participants