[Rev 01] RFR: 6670: Harmonize ~ Unclassifiable ~ across Flame View and Stacktrace View
Marcus Hirt
hirt at openjdk.java.net
Mon Jan 20 13:28:46 UTC 2020
On Mon, 20 Jan 2020 13:28:44 GMT, Miroslav Wengner <mwengner at openjdk.org> wrote:
>> 6670 : Harmonize ~ Unclassifiable ~ across Flame View and Stacktrace View
>> https://bugs.openjdk.java.net/browse/JMC-6670
>
> The pull request has been updated with 1 additional commit.
application/org.openjdk.jmc.flightrecorder.flameview/src/main/java/org/openjdk/jmc/flightrecorder/flameview/tree/TraceTreeUtils.java line 56:
> 55:
> 56: private static TraceNode getRootTraceNode(String rootName, Fork rootFork) {
> 57: return new TraceNode(rootName == null ? DEFAULT_ROOT_NAME : rootName, rootFork.getItemsInFork(),
Is this really necessary? The factory class doesn't add much in terms of abstraction over having a factory method, and it's not clear to me why the get by frame method belongs in there and not the rest of the utility methods. I'd revert these changes, or at least add the helper methods as private statics without the extra inner class.
application/org.openjdk.jmc.flightrecorder.ui/src/main/java/org/openjdk/jmc/flightrecorder/ui/messages/internal/Messages.java line 526:
> 525: public static String VMOperationPage_TIMELINE_SELECTION;
> 526: public static String Flameview_UNCLASSIFIABLE_FRAME_DESC;
> 527:
Since it's only the flame view that uses this description, it could be in FlameView (it really should be, if it is to be named FlameView something). If we want the description to be generally available, it should be somewhere else. If we want it to be available to rules etc, it probably should live somewhere in core, near to where the definition of the unclassifiable frame is, e.g. in flightrecorder. One possible solution, would e.g. be to add a public messages bundle in the stacktrace package.
-------------
Changes requested by hirt (Lead).
PR: https://git.openjdk.java.net/jmc/pull/38
More information about the jmc-dev
mailing list