Sv: RFR: JMC-6555 Convert JOverflow plugin to SWT

[Marcus Hirt]

To all JOverflow users:

Please give this patch a try!

Kind regards,
Marcus

-----Ursprungligt meddelande-----
Från: jmc-dev <jmc-dev-bounces at openjdk.java.net> För Arvin Kangcheng Xu
Skickat: den 10 september 2019 22:02
Till: jmc-dev at openjdk.java.net
Ämne: Re: RFR: JMC-6555 Convert JOverflow plugin to SWT

This is the latest updated patch for fixes of the following, as mentioned earlier in this thread:

 - wildcard imports were replaced with single class imports
 - unnecessary white spaces were removed
 - indentations were changed to using tabs instead of spaces
 - removed mIsUpdatingModel guard
 - removed getHeapSize and mHeapSize in BaseViewer
 - declared setHeapSize in BaseViewer abstract
 - initialized mHeapSize to 1 to avoid division by zero
 - numbers are now rounded instead of truncated
 - number displays are now comma-separated
 - removed global jfx dependencies (javafx.osgi, p2 repo, target platforms)
 - refactored sub-component calls
 - used a more contrasting color palette for pie charts
 - fixed rotating table color
 - memory column displays 2 decimal places. add tooltips
 - fixed JavaThingPage NPE

Spotbug config typos were resolved in another issue and are now fixed and push.

Webrev:
http://cr.openjdk.java.net/~aptmac/JMC-6555/webrev.03/

Thanks to Alex for creating this webrev.

Regards,

On Thu, 29 Aug 2019 at 09:58, Jie Kang <jkang at redhat.com> wrote:
>
> On Tue, Aug 27, 2019 at 1:41 PM Arvin Kangcheng Xu <kxu at redhat.com> wrote:
> >
> > Hello all,
> >
> > The following is the updated webrev. Several issues mentioned in 
> > this thread are addressed:
> > - wildcard imports were replaced with single class imports
> > - unnecessary white spaces were removed
> > - indentations were changed to using tabs instead of spaces
> > - removed mIsUpdatingModel guard
> > - removed getHeapSize and mHeapSize in BaseViewer
> > - declared setHeapSize in BaseViewer abstract
> > - initialized mHeapSize to 1 to avoid division by zero
> > - numbers are now rounded instead of truncated
> > - number displays are now comma-separated
> > - removed global jfx dependencies (javafx.osgi, p2 repo, target 
> > platforms)
> > - refactored sub-component calls
> > - used a more contrasting color palette for pie charts
> >
> > Webrev:
> > https://cr.openjdk.java.net/~jkang/joverflow-swt/webrev.02/
>
> Hi all,
>
> I talked with Arvin through chat for some issues but for archive 
> purposes I will list them, as well as some other issues here:
>
> * The rounding of KB values close to zero makes the data completely 
> lost as the result shown is 0 KB. It would be nice if it had a few 
> decimal points and a tooltip that showed the exact value.
> * When entering and exiting a table entry with mouse, the colour 
> palette now rotates through the colours so they can no longer match 
> the pie chart
> * After the alters to the spotbugs excludes file an error [1] now 
> shows up. The match for org.openjdk.jmc.joverflow.ui.FXMain was 
> written incorrectly with "OR" instead of "Or" so I guess it matched 
> everything. Now that it's gone, a proper exclusion needs to be added 
> to keep spotbugs happy.
>
> * I have opened a JMC instance which automatically re-opens a hprof 
> file I had opened in a previous run of the application. When I use 
> Window -> Show View -> Other -> JOverflow -> JOverflow Instances, I 
> see the instances panel load with an NPE [2]. Note if I then close and 
> re-open JMC where both the hprof file and the instances panel are 
> loaded, this doesn't occur.
> * After seeing the NPE in the instances panel, when clicking around 
> somewhat randomly in the view of a hprof file and repeatedly pressing 
> the "Reset" button in the top-right corner, I see a Problem Occurred 
> pop-up showing an NPE [3]. It occurs consistently when clicking an 
> element in the Object Selection table (top left corner). The "Reset"
> button also no longer causes the views to reset.
> * If I have the Instances panel open before I open a hprof file it 
> seems okay for the most part. I have also checked the previous 
> revision and Similar behaviour also occurs
>
> [1]
>
> [INFO] --- spotbugs-maven-plugin:3.1.10:check (default) @ 
> org.openjdk.jmc.commands --- [INFO] BugInstance size is 1 [INFO] Error 
> size is 0 [INFO] Total bugs: 1 [ERROR] 
> org.openjdk.jmc.commands.internal.executables.Version.execute(Statemen
> t,
> PrintStream) invokes System.exit(...), which shuts down the entire 
> virtual machine 
> [org.openjdk.jmc.commands.internal.executables.Version] At 
> Version.java:[line 47] DM_EXIT
>
> [2]
>
> java.lang.NullPointerException
> at 
> org.openjdk.jmc.joverflow.ui.JavaThingPage.allIncluded(JavaThingPage.j
> ava:80) at 
> org.openjdk.jmc.joverflow.ui.JOverflowUi.updateModel(JOverflowUi.java:
> 141) at 
> org.openjdk.jmc.joverflow.ui.JOverflowUi.addModelListener(JOverflowUi.
> java:158) at 
> org.openjdk.jmc.joverflow.ui.InstancesPageBookView.lambda$0(InstancesP
> ageBookView.java:59) at 
> org.openjdk.jmc.joverflow.ui.JOverflowEditor.addUiLoadedListener(JOver
> flowEditor.java:271) at 
> org.openjdk.jmc.joverflow.ui.InstancesPageBookView.doCreatePage(Instan
> cesPageBookView.java:59)
> [truncated...]
>
> [3]
>
> java.lang.NullPointerException
> at 
> org.openjdk.jmc.joverflow.ui.JavaThingPage.allIncluded(JavaThingPage.j
> ava:80) at 
> org.openjdk.jmc.joverflow.ui.JOverflowUi.updateModel(JOverflowUi.java:
> 141) at 
> org.openjdk.jmc.joverflow.ui.viewers.BaseViewer.notifyFilterChangedLis
> teners(BaseViewer.java:24) at 
> org.openjdk.jmc.joverflow.ui.viewers.OverheadTypeViewer.setCurrentType
> (OverheadTypeViewer.java:90) at 
> org.openjdk.jmc.joverflow.ui.viewers.OverheadTypeViewer.lambda$0(Overh
> eadTypeViewer.java:33) at 
> org.eclipse.jface.viewers.Viewer$1.run(Viewer.java:151)
> [truncated...]
>
>
> Regards,
>
>
> >
> > Thanks to Jie for updating the webrev.
> >