Possible to display time in flame graph?

[brtubb-patagonia]

Is there a concept of time (relative or absolute) available to display in the Call Traces or Back Traces?

Right now I'm going off of relative stack widths to find slow code to optimize. However, when I fix one slow section of code, it's hard to get a picture of how much time was saved.

[kornilova203]

Yes, it's possible to show actual time or at least number of stacktraces (flamegraph files do not contain info about time)
This issue is similar to #23 except that 23 is about popups

[brtubb-patagonia]

@kornilova-l Thanks, that's good to know.

I was trying to do some of the work on this myself, depending on complexity. Having issues building on Windows. Are you doing your development in Linux?

Main issue appears to be a missing package (com.github.kornilova_l.flamegraph.cflamegraph).

[kornilova203]

yup, I use ubuntu and macos
cflamegraph package contains code generated by flatbuffers. There is a gradle task compileFlatBuffers that uses flatc that should be installed beforehand.

I checked that cflamegraph contains only 3 small classes and decided pushed it to repo so now it's not necessary to install flatbuffers.
Try to update project and compile:

gradlew.bat compilePlugin
gradlew.bat runIdea

You may also see unresolved package com.github.kornilova_l.flamegraph.proto. This code will be automatically generated when you run compilePlugin (see protobuf module). And you don't have to install protobuf.

Tell me if you have any other issues.

cflamegraph is flatbuffers binary format for storing flamegraph files (see CompressedFlamegraphToCallTracesConverter).
Flatbuffers is similar to protobuf but it's faster (see protocol buffers vs flatbuffers).
Currently java code reads cflamegraph file and constructs tree using classes generated by protobuf, then it sends serialized tree to client (browser) that deserializes it and does visualization.
But protobuf takes huge amount of time for serialization/deserialization. I plan to replace protobuf with flatbuffers #15
I'll remind myself about project structure and write workflow for fixing this issue.

[kornilova203]

BTW what profiler do you use?

[brtubb-patagonia]

I'm using Java Mission Control, Flight Recorder, for Java 8. Running on Windows Server 2012, which is the cause of some chaffing. I understand if that isn't something you want to bother supporting.

I was able to generate the Protobuf classes without issue. Will give the flatbuffers a try. Thanks!

BTW, there is an issue on Windows with the firerix test files. Files have wildcards in the names.

[brtubb-patagonia]

I see the pre-compiled classes in the latest commits. Thanks! Working on attaching a debugger so I can get a better grasp on it now.

Entry point (File reading -> flatbuffer) is FileToCallTracesConverter.kt?

[kornilova203]

Hi,
You should be able to debug Gradle task runIdea from IDE.

There are two types of converters:

1. FileToCallTracesConverter is called when user clicks on a file name in the list of uploaded files. There are converters for cflamegraph (binary representation of flamegraph) and legacy converters for yourkit and flamegraph (text representation of flamegraph).
2. ProfilerToCompressedFlamegraphConverter is called when user uploads a file. It converts the file to cflamegraph so we do not need to extract data from original file all the time when we want to see flamegraph. For instance, yourkit files are converted to cflamegraph by YourkitCsvToCFlamegraphConverter when file is uploaded. And when user clicks on a file call traces are constructed by CompressedFlamegraphToCallTracesConverter

Also there is a ProfilerToFlamegraphConverter that should be deprecated. It does almost the same thing as ProfilerToCompressedFlamegraphConverter but it outputs text representation of flamegraph. Text representation consumes a lot of space on disk and also it takes much more time to parse it. And this is the way jfr files are handled right now.

It would be nice if you change JMCConverter such that it implements ProfilerToCompressedFlamegraphConverter so we get rid of regular text flame graphs.
It's good to do it first because otherwise we will need to edit both FlamegraphToCallTracesConverter and CompressedFlamegraphToCallTracesConverter.

I hope that you will not be too confused by all these converters. FlameViewer was my first complex project and I did not predict performance issues related to text flamegraphs.

Now I will create issue for converting jfr directly to cflamegraph and also I'll deprecate code related to text flamegraphs.

[kornilova203]

Note that we cannot remove deprecated FileToCallTracesConverter implementations because some of users may already have uploaded files that were not converted to cflamegraph. But we can remove, for example, ProfilerToFlamegraphConverter because it's called only when user uploads a file.

[kornilova203]

Here is new related issue: #48
It would be cool if you take a look at it!

[brtubb-patagonia]

Thanks! I'll take a look. I'm juggling a couple things at work, so when I get a breather I'll poke around. If I'm comfortable enough with the changes I'll submit a PR.

[kornilova203]

Sure, no worries :)
I've just pushed couple of commits, don't forget to update

[brtubb-patagonia]

What build task(s) should I run before runIdea? I see the debugger attach (circle with the check) to the .kt code when I debug runIdea, but see a 404 in the browser. I think I'm missing some gradle steps, perhaps for static files.

I'm running build in the Flameviewer root,

Thanks for the help getting it running!

[brtubb-patagonia]

I'm excluding tests right now due to the files I can't download (Windows). Using -x test.

[brtubb-patagonia]

I was able to get everything working after running ~15 promising tasks. Not sure which one did it, but it's working!

[brtubb-patagonia]

Ok, so I took a crack at moving to Compressed Flamegraphs for JFRs. Didn't go very well. I was able to avoid compiler errors, but getting a failure when moving the file to the cflamegraph folder. Will have to check later on the permissions and compare to a .ser file. Not seeing an exception thrown so I couldn't dig in.

I did find promising news on adding a time dimension to the flamegraphs. When parsing the JFR file (FlightParser.buildStacks), the UTC epoch ns is available on each frame. These JFRs are just samples, so a naive implementation would be numberOfSamples * sampleTime. That would require an update to your FlightParser class, which is just a HashMap<String, Integer> and probably not expressive enough.

This would only work for sampled JFRs. No idea how to handle other types of profiling techniques which I think JFR supports.

[kornilova203]

Task that prepares static files and compiles javaagent used to be compilePlugin but yesterday I renamed it to prepareStaticAndAgent.

Yup, probably we will add optional time field to cflamegraph, but it's a different task.

You can create a [WIP] PR, so I can take a look!

[kornilova203]

Hi, @brtubb-patagonia
Here is a task that you may do :)

Steps to display time on popup:

1. Add <p class="duration"></p> block to accumulativeTreePopup template in tree-templates.soy file (similar to callTreePopup template)
2. Move code that adds duration to popup from CallTreeDrawer._setPopupContent to base method in TreeDrawer (not that time unit "ms" should be displayed only in call trees, so it's a good idea to add method _getTimeUnit that returns an empty string and override it in CallTreeDrawer)
3. Run copyStatic gradle task
4. See if it worked

Note that the displayed number will be either milliseconds (for yourkit files) or number of stacktraces (jfr files and uploaded flamegraph files).

Issue about retrieving time from jfr: #54

Maybe we will add an optional field measure unit to cflamegraph, then we will be able to display it on the popup. But now I want to keep cflamegraph as simple as possible

[brtubb-patagonia]

@kornilova-l Sorry for the delay, just getting back from vacation. I hope your trip to Finland went well!

I''ll jump on these tasks this week. Once I get through these, do you want it as a PR?

[kornilova203]

Yup, a PR to master :)

[brtubb-patagonia]

Still working on this... I've been having merge issues due to the invalid characters (under Windows) for some test files. I was able to resolve under WSL. Back at it :)

[brtubb-patagonia]

I was able to get this working. Before I submit a PR, I want to run this by you.
With this change, only a number is displayed, which is confusing. I added 'samples' as the override in AccumulativeTreeDrawer. Would you be ok with this addition?

I don't have any yourkit files, so if you could double check before accepting a PR that'd be great!

[kornilova203]

Good! Today I'll check commit in your master branch and think about 'samples'
You still may submit a PR, I'll will not merge it without making sure that it works :)

[kornilova203]

I tested your master branch, it works fine. I saw number of milliseconds from yourkit csv file.
I think we should not add samples or anything else right now. The problem is that frontend is not aware of file contents, time unit should be received from backend.

BTW: there are some profiler files in `/src/test/resources/plugin/server/trees/profiler-files` (`csv` dir contains yourkit files)

[brtubb-patagonia]

That makes sense, I'll submit a PR of master into the jfr branch you have. I agree - timeunits should really come from the backend.

[kornilova203]

Submit to master