Heatmap converter #944

Artyomcool · 2024-06-16T17:56:09Z

This PR adds Heatmap html file format to async-profiler converters collection.

More about heatmaps: https://www.brendangregg.com/heatmaps.html

This version simpler, but slower, then our previous one: https://github.com/Artyomcool/async-profiler/tree/heatmap
Basically, I've removed all "optimizations" (dirty hacks) from JFRReader, some of which had potential issues.

apangin

Thank you very much! The feature looks very promising.
I haven't looked through the logic yet, just a few overall comments.

src/converter/one/util/Frames.java

src/converter/one/util/Index.java

src/converter/one/util/ResourceProcessor.java

src/res/heatmap.html

src/converter/one/convert/JfrToHeatmap.java

src/converter/one/heatmap/Heatmap.java

src/converter/one/util/DictionaryInt.java

src/converter/one/util/ResourceProcessor.java

src/res/heatmap.html

apangin

Thanks for addressing my previous comments. I looked a bit deeper and added a few more questions/comments. Not deep enough yet to comment on the algorithm itself :)

Please, rebase the PR on top of the current master, otherwise, while testing the converter I get errors that have been already fixed. Thank you.

src/converter/Main.java

src/converter/one/convert/JfrToHeatmap.java

src/converter/one/convert/FlameGraph.java

src/converter/one/heatmap/Heatmap.java

apangin · 2024-07-20T23:21:00Z

src/converter/one/heatmap/Heatmap.java

+        state.methodsCache.assignConstantPool(methodRefs, classRefs, symbols);
+    }
+
+    public void nextFile() {


Is it really used for multi-file processing? Why not just creating new Heatmaps instead?

It allows mixing several jfr files into one single heatmap, and sometimes it is very useful.

apangin · 2024-07-21T00:09:39Z

src/res/heatmap.html

+
+		nextSymbol() {
+			return this.byteAt(this.pos++);
+		}


This is a verbatim copy of nextInt6. Maybe, add nextBase123 instead?

Yeah, looks strange) Maybe somebody else took my body to code this peace)

apangin · 2024-07-21T00:13:23Z

src/res/heatmap.html

+			return this.nextInt6() | this.nextInt6() << 6 | this.nextInt6() << 12;
+		}
+
+		int30(pos) {


Instead of this function, I'd just set pos to the tail of the stream and reuse nextInt30.

I don't think touching pos is a good idea, nice place for bugs.

apangin · 2024-07-21T00:17:33Z

src/res/heatmap.html

+					let runAgain;
+					try {
+						runAgain = task.f();
+					} catch (e) {


What exception can it be?

apangin · 2024-07-21T00:19:31Z

src/res/heatmap.html

+						nextTask = 0;
+						overflow = true;
+					}
+				} while (performance.now() - start < 8);


Just wondering about this magic value

Numbers from 4 to 20 work nice)))
Big numbers make UI stuck, low numbers make calculations slow.

I'll make some named constant

apangin · 2024-07-21T00:28:40Z

src/converter/one/heatmap/Heatmap.java

+    private void compressMethods(HtmlOut out, Method[] methods) throws IOException {
+        for (Method method : methods) {
+            out.write18(method.className);
+            out.write18(method.methodName);


So, the conversion will fail if there are more than 2^18 methods?
Also, will it be more compact to use varint encoding for classess, methods and locations?

It can be more compact, yes, but using var length encoding may lead to extra preprocessing step on the html side. I'll check if it is the case.
But it should be more then 2^18 unique method names (not including classes). Anyway, we don't need to be compact here, but it would be nice to be safe)

apangin · 2024-07-21T00:50:04Z

src/converter/one/jfr/Index.java

+            if (currentKey == null) {
+                break;
+            }
+            //noinspection unchecked


This is an IDE-specific hint. javac will still issue a warning. Please use @SuppressWarnings instead.

moreover, it is unneeded in current implementation, fixed

apangin · 2024-07-21T03:24:05Z

I noticed on many profiles that a heatmap gets truncated when switching to a larger scale:

E.g., on 1 minute scale, the heatmap ends at 14:19:42

while on 5 minute scale, it ends at 13:19:55

apangin · 2024-07-21T03:35:12Z

There is inconsistency in time formats: 24h format on the top and 12h format on a tooltip. Can it be always 24h?
Also, on the top ruler, time is printed in the middle between ticks, so it's unclear which tick it corresponds to.

Artyomcool · 2024-07-22T10:27:07Z

Thank you very much for your review. I'll fix the bugs with canvas (looks like it happened after merging dev on some point).

Artyomcool · 2024-08-10T08:26:54Z

Heatmap rendering bugs fixed as well

rus-99-pk · 2024-09-16T04:40:02Z

Hello!

I tried to test heatmap, and it works nice.

But when i tried to merge JFR's and convert it ho heatmap, i stuck on exception:

$ jdk-21.0.1+12/bin/java -jar jfr-converter.jar -o heatmap /tmp/file.jfr /tmp/test.html
Converting file.jfr -> test.html Exception in thread "main" java.lang.ArrayIndexOutOfBoundsException: Index 0 out of bounds for length 0
	at one.convert.JfrConverter.convertJavaClassName(JfrConverter.java:139)
	at one.heatmap.SymbolTable.convertClassName(SymbolTable.java:157)
	at one.heatmap.SymbolTable.orderedKeys(SymbolTable.java:147)
	at one.heatmap.MethodCache.orderedSymbolTable(MethodCache.java:157)
	at one.heatmap.Heatmap.evaluate(Heatmap.java:72)
	at one.heatmap.Heatmap.dump(Heatmap.java:89)
	at one.convert.JfrToHeatmap.dump(JfrToHeatmap.java:70)
	at one.convert.JfrToHeatmap.convert(JfrToHeatmap.java:81)
	at Main.convert(Main.java:60)
	at Main.main(Main.java:46)

More details here.

Artyomcool · 2024-09-17T14:40:31Z

@rus-99-pk pls, provide original jfrs and the merged one

rus-99-pk · 2024-09-19T09:13:27Z

@Artyomcool, was send at upload@profiler.tools.

Artyomcool · 2024-09-20T16:54:53Z

@rus-99-pk fixed
Thank you for testing!

Signed-off-by: Artyom Drozdov <artyomd@lightrun.com>

Artyomcool · 2024-10-09T09:56:34Z

@rus-99-pk since you are voluntarily testing the feature, be aware, the very critical bug was fixed with the last commit

# Conflicts: # src/converter/one/convert/JfrConverter.java

Technically, we could use 0 on the encoder side, but by passing -1 we can distinguish frames with no line info provided and line info irrelevant.

Artyomcool added 3 commits June 16, 2024 19:48

Heatmap converter

bfe1abd

No debug output

41dd634

Overflow

6e0841c

apangin reviewed Jun 22, 2024

View reviewed changes

Artyomcool added 3 commits July 7, 2024 18:17

Review fixes

b044a40

Review fixes

5d4a9e2

Review fixes

30e5ece

Artyomcool requested a review from apangin July 9, 2024 09:24

apangin reviewed Jul 21, 2024

View reviewed changes

Artyomcool added 3 commits August 9, 2024 23:43

Fixes

4211b1c

Method cache refactoring byte[] -> String

54829db

Review fixes

ed86d5e

Artyomcool force-pushed the heatmap-2 branch from 7d79bda to ed86d5e Compare August 10, 2024 08:25

Artyomcool requested a review from apangin August 12, 2024 10:25

Start/end encoding safety markers

0ad9ce4

Artyomcool mentioned this pull request Sep 13, 2024

Change flame graph ordering of frames from by-name to by-total #990

Closed

apangin mentioned this pull request Sep 13, 2024

Add a JFR merge feature #991

Closed

2 tasks

JFR stackTraces clean-up

61fe0d5

Artyomcool added 3 commits September 20, 2024 19:15

Unneeded import

223bf54

Fix ignored value in aggregates

d4a8011

Merge branch 'master' into heatmap-2

3a9646e

Signed-off-by: Artyom Drozdov <artyomd@lightrun.com>

apangin and others added 14 commits December 27, 2024 22:16

Merge branch 'master' into heatmap-2

322290d

# Conflicts: # src/converter/one/convert/JfrConverter.java

Integrate with new design of JfrConverter. Minor restyling.

2ced7c3

Deduplicate Index

15e9b98

Remove unused methods. Check for empty output.

aeea94e

SymbolTable removal

8aa62b3

Initial sizes for hashmaps

ebce852

Ctrl+C/V issue)

b659500

-1 issue

6724498

Corrected project URL

d8f9ca5

Fix for frames with irrelevant line information

41ba84d

Technically, we could use 0 on the encoder side, but by passing -1 we can distinguish frames with no line info provided and line info irrelevant.

Refactor JfrConverter for better extensibility

06a05fc

Merge branch 'master' into heatmap-2

872567d

Code cleanup: reuse JfrConverter for class/method name resolution

129c5d2

Cleanup

d1bb78c

apangin approved these changes Dec 29, 2024

View reviewed changes

apangin merged commit ec83ae6 into async-profiler:master Dec 29, 2024
10 checks passed

apangin mentioned this pull request Jan 2, 2025

d3 only visualise found stackframes #593

Closed

Heatmap converter #944

Heatmap converter #944

Uh oh!

Conversation

Artyomcool commented Jun 16, 2024

Uh oh!

apangin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

apangin left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

apangin commented Jul 21, 2024

Uh oh!

apangin commented Jul 21, 2024

Uh oh!

Artyomcool commented Jul 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Artyomcool commented Aug 10, 2024

Uh oh!

rus-99-pk commented Sep 16, 2024

Uh oh!

Artyomcool commented Sep 17, 2024

Uh oh!

rus-99-pk commented Sep 19, 2024

Uh oh!

Artyomcool commented Sep 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Artyomcool commented Jul 22, 2024 •

edited

Loading

Artyomcool commented Sep 20, 2024 •

edited

Loading