I also think it would be a nice improvement to MAT to add some more automatically shown warnings.
>> I think we can split the problem into 4 parts.
+1
>> 1.Examples of warnings
In the last days I also thought that we should first collect some more concrete examples of what problems we could tackle.
My biggest concern about doing some analysis per object is the performance. Let’s try to collect some ideas of concrete warnings, which can be
a) calculated based on (or starting with) a single object and b) calculated very fast. I think the example from Kevin with the big object, and from Andrew with the disposed graphics are a good start. What else could we collect?
-
Big objects (> % of the total heap)
-
Closed files
-
Un-/disposed widgets
https://bugs.eclipse.org/bugs/show_bug.cgi?id=274369
-
Small Strings objects, keeping a huge char[] (as result of substring() call)
-
A big, empty collection ?? - not sure if this is useful on a single object. The reports showing millions of empty collections are good,
but on a single object I don’t know.
We had some ideas in the past to show WeakHashMap objects, which contain entries where the value is referencing (may be indirectly) the key. This
however requires some path analysis, and is one of the warnings which I wouldn’t calculate for every opened view containing a WeakHashMap.
>> 2.How should the warnings be indicated to the user?
I find both proposals good - coloring the text or using a marker on the icon.
>> Would showing details of the warning in a tab in the object inspector be a good location?
+1
>> Would an extension point to specify the icon for an object be useful?
I think so.
I’ll think a bit more about the other questions.
About performance:
Depending on the concrete problems we identify in 1) it may help to run a scan once, and save the results somehow, so that opening a view later
does not result in new computations.
I wouldn’t do it in the parsing phase, or at least it should be possible to turn such additional analysis off.
It covers to some extent what Kevin proposed:
>> An additional report could exist on the Overview screen under Reports named "Show Any Warnings." This would require finding all warningResolver
extensions and passing each instance of the subject classes to check for warnings.
And it is also similar to the current component reports, but there we do not persist the results to mark objects afterwards.
I enjoy this discussion, let’s go on
J
Krum
From: mat-dev-bounces@xxxxxxxxxxx [mailto:mat-dev-bounces@xxxxxxxxxxx]
On Behalf Of Andrew Johnson
Sent: Freitag, 1. März 2013 11:50
To: Memory Analyzer Dev list
Subject: Re: [mat-dev] Proposal: MAT Warning System
I agree that adding more automatic warnings would make MAT more useful for the inexperienced user.
As is, I don't think it is quite the right approach. I think we can split the problem into 4 parts.
1.Examples of warnings - so we can see if warnings are globally applicable, or are complex to calculate. E.g. the warning is per object, so the evaluation needs to be speedy, but also accurate.
For example warning about closed files or disposed graphics objects might make sense if they were being retained when no longer needed, but in another situation excessive numbers of unclosed files could indicate a resource leak.
2.How should the warnings be indicated to the user?
The coloured coded row (background or foreground?) seems a convenient way to indicate a problem, but there are others.
See also QueryPart IS_IMPORTANT which highlights a section in a report, and the focus row in blue in a report.
A standard Eclipse way is to use an overlay (a decorator) on the icon. For example a warning triangle or red error cross on projects which are in error. Would that be more intuitive? Should we
then have a problems view? You could then show warnings/errors on a selection or across the dump, and group the errors by type, then jump to an instance of the error. These are markers - see IMarker. We could also allow the user to annotate an object with
some text (that would be visible in other views), or possibly tasks (e.g. TODO items for objects the user wants to hand inspect). We could also have a preferences page - should there be a per snapshot preferences for warnings? How might that be done - should
that be like per project Java warnings. Should snapshots map to Eclipse projects in the projects explorer? Should we make it easy to have multiple workspaces for MAT? E.g. all snapshots for one end-user application could be in one workspace, and then by switching
workspaces you could then see all the snapshots from another application.
Would showing details of the warning in a tab in the object inspector be a good location?
Would an extension point to specify the icon for an object be useful?
3.How should adopters write warning extensions?
The IClassSpecificWarningResolver seems a reasonable start, though markers seem a good model and could allow the linking of help.
4.How should it be implemented internally?
Doing icon overlays in org.eclipse.mat.api or report is harder, as currently they does not use the Eclipse UI classes. That was important when AIX didn't have a 64-bit UI, but might be less so
now.
Other comments:
1.Internals/adopters
The name resolver implementation has a slight problem that only one resolver per class is allowed. Name resolvers then work from most specific to least specific. I had an idea for IBM PHD/javacore
dumps of using a name resolver for java.lang.Threads from DTFJ instead of the standard resolver using the name field.
2.Adding methods to IObject is not a compatible change - we could have a new interface, but alternative implementation of IObject would need to implement that method to take advantage of this idea,
which doesn't really need the implementation specifics of the IObject.
3.Why tree view and not table view as well?
4.How do we save/display the warning text in HTML for saved reports?
5.@Subject("java.lang.Object") would probably do the same as "*".
Andrew Johnson
From: Kevin Grigorenko <kevin.grigorenko@xxxxxxxxxx>
To: mat-dev@xxxxxxxxxxx,
Date: 25/02/2013 16:16
Subject: [mat-dev] Proposal: MAT Warning System
Sent by: mat-dev-bounces@xxxxxxxxxxx
Proposal for an extensible warning system in MAT:
1. A new extension point named org.eclipse.mat.api.warningResolver. In a similar fashion to nameResolver, a class would be annotated with @Subject and implement an interface IClassSpecificWarningResolver with a method named resolveWarning. This would return
an instance of a class WarningResolution which has two items: an enumeration value (Warning would be the only choice initially) and a "details" String.
2. A method is added to IObject: List<WarningResolution> getClassSpecificWarnings();
3. In a tree view, in the same way that any nameResolvers are called for each row, any warningResolvers would be called for each row. If a warningResolver returns a Warning result, the row is color coded. If there are multiple warning resolvers, the highest
enumeration value is the one chosen. By default, the color for warning would be red, but there should be a preference to choose the color. Additionally, the context menu is modified to add an item at the top called "Warning Details." When clicked, this raises
a dialog with the "details" Strings returned from the warningResolvers previously.
4. In an HTML view, any tree views converted to HTML tables should also have color coded rows, if applicable.
5. There is a special subject @Subject("*") which instructs MAT to call it's resolveWarning method for every class. This could be used by class-independent warning analysis, for example: Warn if an object's retained heap is > x% of total heap size.
6. A set of preferences are added: A checkbox to completely enable or disable the warning system (enabled by default) along with a way to choose the color for a warning.
Let me know what you think. If the proposal is good, I'll start working on a patch and send it to the group for review when ready. Also, this is changing the API (adding an extension point and adding a method to IObject) -- does that affect versioning, etc.?
Some other thoughts:
1. I thought about making IClassSpecificWarningResolver extend IQuery, and then clicking the warning details context menu item would run an instance of IClassSpecificWarningResolver to show a normal query. The problem is that this would be complicated to deal
with if there are two or more warning resolvers for the same class and it would complicate the implementation for extensions.
2. An additional report could exist on the Overview screen under Reports named "Show Any Warnings." This would require finding all warningResolver extensions and passing each instance of the subject classes to check for warnings.
3. There could also be a preference to automatically run the report in #2 when loading a heapdump and show any warnings in a section in the Overview report.4. Options #2 and #3 could of course be very expensive so maybe an extension marks itself somehow as
able to check for important warnings in some lightweight way, perhaps through a separate method call instead of passing all object instances to resolveWarning.
4. Options #2 and #3 could of course be very expensive so maybe an extension marks itself somehow as able to check for important warnings in some lightweight way, perhaps through a separate method call instead of passing all object instances to resolveWarning.
--
Kevin Grigorenko
IBM WAS SWAT Team
kevin.grigorenko@xxxxxxxxxx
Blog: https://www.ibm.com/developerworks/mydeveloperworks/blogs/kevgrig/?lang=en_______________________________________________
mat-dev mailing list
mat-dev@xxxxxxxxxxx
https://dev.eclipse.org/mailman/listinfo/mat-dev
Unless stated otherwise above:
IBM United Kingdom Limited - Registered in England and Wales with number 741598.
Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
