Disable disassembly of inline functions

[lievenhey]

Currently this doesn't work correctly. In the flamegraph the top entry doesn't has the inline flag set.

[milianw]

can we get a test too please? :)

[lievenhey]

The second commit is just for debugging. I will remove it once everything works. Done

[lievenhey]

I also added a color scheme to the flamegraph to show which functions are inlined.

[milianw]

Cool, can you also add the combined mode, that shows system code / user code / inlined code? I think that is going to be very useful

[milianw]

OK, you can rebase, I fixed the issue with the duplicate inline frame now

[lievenhey]

I updated the brushed so that they always show if a frame got inlined

[lievenhey]

I also had to remove the costBrush since:

• it wasn't really useful
• it broke and created colors that ware outside the hsv scope

[lievenhey]

the ci not working is expected, since I updated it to kddw 2.0 this morning. It will work once #570 is landed

[GitMensch]

@lievenhey This seems to still need an automated test of some sort, right?
Apart from that this seems ready to merge and looks very useful.

[milianw]

Hey @lievenhey - I revamped the colorization of inline frames in the flamegraph, and include the info now also in tooltips. Can you give it another shot and see if that works for you too?

From my side, this is otherwise fine now.

[GitMensch]

This sounds good to me, but it looks like adjustment to README / doc pngs are "missing".
Of course I'm fine with both handling those here as well as a follow-up PR.

[GitMensch]

From my side, this is otherwise fine now.

Note @milianw: at least from the PR state it still has "change requested" from you.

[GitMensch]

I've tinkered with the appimage, and found the flame-graph to be quite reasonable now (much better than the previous one).
... but: I'm not sure about the inline function costs and the disassembly.

Of course, there is no option any more to use the "Disassembly" context on the inlined functions, which is fine, but I can't see neither the costs in the Disassembly of the caller nor the Disassembly for it either:

Shouldn't I see those 25.8% in line 418?

Note: when I use the caller view, I can "get into" the function, this shows:

but currently I have to manually "overlay" the cpu cycles % over the source view and seem to have no option to reach the actual disassembly details (the disassembly is there, of course).

[milianw]

@GitMensch please open a new issue for whatever you are seeing there, and reproduce the issue in a way we can replicate it ourselves. As is there's not enough information for me to give you an answer to your question.

[GitMensch]

Hm, that likely takes a while... short summary: the disassembly of inline functions has NO cost at all with this version, while it should have the cycles incl. set to the one that is visible in the Caller/Callee tab.

Note: also stumbled over #577 again (a relative new, but really bad bug).

[milianw]

How do you get disassembly of an inline function to begin with? that shouldn't be possible anymore after all?!

[GitMensch]

How do you get disassembly of an inline function to begin with? that shouldn't be possible anymore after all?!

as noted: the disassembly of one function included the disassembly for a good part of the source, including (one of the generations of) the inline function. Showing the disassembly or not here isn't the main point I have with the effect of this PR, it is that the inlined function has no cost shown in the source code window anymore (you only see that by the caller/callee tab)...

[milianw]

the inline symbol shouldn't even show up in the disassembly view at all, thus it not showing any cost there is to be expected, no? after all, we cannot open the disassembly for that anymore... I really don't follow what the issue is here, there seems to be a fundamental misunderstanding between us on how things should be working...

[lievenhey]

Hey @lievenhey - I revamped the colorization of inline frames in the flamegraph, and include the info now also in tooltips. Can you give it another shot and see if that works for you too?

From my side, this is otherwise fine now.

thanks, I suck at choosing color schemes.

[GitMensch]

the inline symbol shouldn't even show up in the disassembly view at all, thus it not showing any cost there is to be expected, no? after all, we cannot open the disassembly for that anymore... I really don't follow what the issue is here, there seems to be a fundamental misunderstanding between us on how things should be working...

I try to explain what I've thought to understand and what I think should be the UI for that.

• "real" inline (the compiler is free to inline or not after all) means that the compiler includes the code for a function within another function
• the actual generation (and therefore the diassembly) may be different in each place concerning the context (for example a compiler may recognize that specific parts can only be taken in specific cases and remove the other parts)
• perf should trace the execution of that as it is tracing non-inlined code

Running objdump shows the disassembly for the "outer" function, therefore the inlined code is part of the disassembly, possibly multiple times (= we can "see" the specific generation for this specific place).
Code-wise we see a function call.

If we can deduce the place of that disassembly to be part of the "calling" place (where it is inlined) then we can directly attribute the costs there.

If we can't do it, then we still have the "original" place (source reference and/or function name) and on the other hand we have the costs in the function of "calling" it (this is something that can be seen in the caller/callee tab, also with this PR applied).
Ideally we'd then be able to attribute the "summed" costs for the disassembly to the calling place -but I do recognize that in this case we'd have to match that in the code, which is not possible to always get correct because of the preprocessor work (for example macros) - we shouldn't start that path.

This leaves the question: Is there a way to deduce the "calling" source reference for functions that are inlined? If we yes, then we should attribute the total costs to that place.

[lievenhey]

I created an issue for this #586