Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [mat-dev] Proposal: Add option to display bytes in KB, MB, GB, or Smart formats

Here's an updated patch in case you want to try with x-internal=true: https://bugs.eclipse.org/bugs/attachment.cgi?id=245453&action="">
I also integrated the previous feedback to show B with bytes in Smart mode (so that things are consistent), and I converted the Format object in BytesFormat to a ThreadLocal to reduce object creation.

--
Kevin Grigorenko
IBM WebSphere Foundation SWAT Team
kevin.grigorenko@xxxxxxxxxx
Blog: https://www.ibm.com/developerworks/mydeveloperworks/blogs/kevgrig/?lang=en


Inactive hide details for Kevin Grigorenko---07/23/2014 02:34:22 PM---Hi Andrew (and Krum), What if I move these classes into aKevin Grigorenko---07/23/2014 02:34:22 PM---Hi Andrew (and Krum), What if I move these classes into an internal package and add this to


    From:

Kevin Grigorenko/Austin/IBM@IBMUS

    To:

Memory Analyzer Dev list <mat-dev@xxxxxxxxxxx>

    Date:

07/23/2014 02:34 PM

    Subject:

Re: [mat-dev] Proposal: Add option to display bytes in KB, MB, GB, or Smart formats

    Sent by:

mat-dev-bounces@xxxxxxxxxxx




Hi Andrew (and Krum),

What if I move these classes into an internal package and add this to MANIFEST.MF?


Export-Package
: org.eclipse.mat,
...
org.eclipse.mat.query.internal;
x-internal:=true,

A quick test shows that this works and resolves API warnings and does not require @since tags or updating the MAT version.


The one downside I see is that if someone has a customized IResultTree with byte columns, they wouldn't be able to specify the proper column type, but that could just be added for 1.5.

--
Kevin Grigorenko
IBM WebSphere Foundation SWAT Team
kevin.grigorenko@xxxxxxxxxx
Blog:
https://www.ibm.com/developerworks/mydeveloperworks/blogs/kevgrig/?lang=en


Inactive hide details for Andrew Johnson ---07/07/2014 11:12:04 AM---API additions require the minor version to be incremented.Andrew Johnson ---07/07/2014 11:12:04 AM---API additions require the minor version to be incremented. http://wiki.eclipse.org/Version_Numbering
    From:

Andrew Johnson <andrew_johnson@xxxxxxxxxx>
    To:

Memory Analyzer Dev list <mat-dev@xxxxxxxxxxx>
    Date:

07/07/2014 11:12 AM
    Subject:

Re: [mat-dev] Proposal: Add option to display bytes in KB, MB, GB, or Smart formats
    Sent by:

mat-dev-bounces@xxxxxxxxxxx




API additions require the minor version to be incremented.
 
http://wiki.eclipse.org/Version_Numbering#When_to_change_the_minor_segment 

Normally service releases don't have API changes.
 

The Memory Analyzer project generally increments the minor version for each annual release (the project, feature and plug-in version numbers don't have to increment in step, but we have done that for releases so far).
 

I'm not sure of the best way around this - wait until near the next release to make this available; branch the code now so that the Luna updates are just service updates and make changes to trunk; make the changes now in a way that doesn't add APIs then move/change them to APIs later.
 

Andrew Johnson
 



From:        
Kevin Grigorenko <kevin.grigorenko@xxxxxxxxxx> 
To:        
Memory Analyzer Dev list <mat-dev@xxxxxxxxxxx> 
Date:        
01/07/2014 19:38 
Subject:        
Re: [mat-dev] Proposal: Add option to display bytes in KB, MB, GB, or Smart formats 
Sent by:        
mat-dev-bounces@xxxxxxxxxxx 




I'm trying to apply this patch to the latest 1.4 code and I'm running into some API version issues. I set the API baseline to MAT 1.4, and applied the patch. The build error is:

The minor version should be incremented in version 1.4.0, since new APIs have been added since version 1.4.0 MANIFEST.MF /org.eclipse.mat.report/META-INF line 6 Version Numbering Problem

I tried to change the Bundle-Version to 1.4.1.qualifier and updated the @since tags in the new classes to 1.4.1, but it continues to complain as it seems to want to update the version to 1.5.0.qualifier.

If I update Bundle-Version to 1.5.0.qualifier and update the @since tags to 1.5, everything works. Is this expected? Is this what I should do?

--
Kevin Grigorenko
IBM WebSphere Foundation SWAT Team
kevin.grigorenko@xxxxxxxxxx
Blog:
https://www.ibm.com/developerworks/mydeveloperworks/blogs/kevgrig/?lang=en


Inactive hide details for Kevin Grigorenko---06/27/2014 11:04:46 AM---Hi Krum, Thanks for taking a look. > It gets a bit more dKevin Grigorenko---06/27/2014 11:04:46 AM---Hi Krum, Thanks for taking a look. > It gets a bit more difficult to read if I have a list of entrie 

    From:
     

    Kevin Grigorenko/Austin/IBM@IBMUS
     

    To:
     

    Memory Analyzer Dev list <mat-dev@xxxxxxxxxxx>
     

    Date:
     

    06/27/2014 11:04 AM
     

    Subject:
     

    Re: [mat-dev] Proposal: Add option to display bytes in KB, MB, GB, or Smart formats
     

    Sent by:
     

    mat-dev-bounces@xxxxxxxxxxx





Hi Krum, Thanks for taking a look.

> It gets a bit more difficult to read if I have a list of entries sorted by size
> And I found it pretty difficult to read in views where the objects are not sorted by size
> Therefore I would suggest to stick to normal bytes as default, and give the users the option to change

That makes sense and I agree, I will change the default to Bytes. One other idea would be to add a "Default" option which uses Bytes in most places and Smart in places such as Pie chart and leak suspects - what do you think of that as a default?

> When using “smart” the bytes do not have a unit displayed. This may be confusing. Still if I have only bytes, I would prefer leaving the unit away (less info). Do you think you could make it this way?

Yes, I will modify to do that.

> I think this is enough. Just let’s discuss a bit more on the couple of remarks about the Smart numbers.

Great, thanks! I will make the modifications above and submit a new patch and also give some time for others to provide thoughts if they'd like.

--
Kevin Grigorenko
IBM WebSphere Foundation SWAT Team
kevin.grigorenko@xxxxxxxxxx
Blog:
https://www.ibm.com/developerworks/mydeveloperworks/blogs/kevgrig/?lang=en



Inactive hide details for "Tsvetkov, Krum" ---06/27/2014 02:26:47 AM---Hi Kevin, after going quickly through the code changes I"Tsvetkov, Krum" ---06/27/2014 02:26:47 AM---Hi Kevin, after going quickly through the code changes I applied them locally and played with them. 

    From:
     

    "Tsvetkov, Krum" <krum.tsvetkov@xxxxxxx>
     

    To:
     

    Memory Analyzer Dev list <mat-dev@xxxxxxxxxxx>
     

    Date:
     

    06/27/2014 02:26 AM
     

    Subject:
     

    Re: [mat-dev] Proposal: Add option to display bytes in KB, MB, GB, or Smart formats
     

    Sent by:
     

    mat-dev-bounces@xxxxxxxxxxx





Hi Kevin,

after going quickly through the code changes I applied them locally and played with them.

I find the change very nice and would encourage you to submit it. I still have some remarks:


>> * The default option is BytesDisplay.Smart which changes the default MAT display behavior (which would be BytesDisplay.Bytes).
 
-      
I really liked (found it easier to read) how the values are rendered with the Smart option in the Pie chart and in the Leak suspects text, i.e. where we have just one or a few values
-      
It gets a bit more difficult to read if I have a list of entries sorted by size, e.g. the Dominator Tree
-      
And I found it pretty difficult to read in views where the objects are not sorted by size, e.g. different reference chains. Having only bytes gave me immediate overview which the big objects are (just the optical length of the number, I don’t really have to read it). However, having a mixture of the different sizes I need to read more or less every single one to spot a bigger object. Here is an example: 
 

Class Name                                                                                       | Shallow Heap | Retained Heap
--------------------------------------------------------------------------------------------------------------------------------
org.eclipse.mat.parser.internal.SnapshotImpl @
0xc230d258                                        |           64 |       7,11 MB
|- source org.eclipse.mat.parser.model.ClassImpl @
0xc02fbe60                                    |          104 |           248
|  '- data org.eclipse.swt.widgets.TreeItem @
0xc31b55c8                                         |           88 |           112
|     '- [3] org.eclipse.swt.widgets.TreeItem[4] @
0xc31b90e0                                    |           32 |           120
|        '- items org.eclipse.swt.widgets.Tree @
0xc2107e30                                      |          288 |       1,88 KB
|           |- [58], [59], [60] org.eclipse.swt.widgets.Control[1024] @
0xc22787d8               |      4,02 KB |       6,69 KB
|           |  '- controlTable org.eclipse.swt.widgets.Display @
0xc1e21e60 Native Stack         |          624 |     146,71 KB
|           |- control org.eclipse.swt.accessibility.Accessible @
0xc21083d0                     |          160 |       1,33 KB
|           '- Total: 2 entries                                                                  |              |              
|- snapshot org.eclipse.mat.ui.snapshot.editor.HeapEditor$SnapshotEditorInput @
0xc204e980       |           32 |           112
|- snapshot org.eclipse.mat.internal.snapshot.inspections.DominatorQuery$DefaultTree @
0xc2cc8388|           40 |            40
|- snapshot org.eclipse.mat.snapshot.query.ObjectListResult$Outbound @
0xc31774c0                |           24 |            24
--------------------------------------------------------------------------------------------------------------------------------
 
 

-      
Therefore I would suggest to stick to normal bytes as default, and give the users the option to change. This is just my opinion, I’d be happy to hear some other comments on this.
-      
When using “smart” the bytes do not have a unit displayed. This may be confusing. Still if I have only bytes, I would prefer leaving the unit away (less info). Do you think you could make it this way? 

>> * The additional instances of the Bytes class might add some performance overhead.


I think these are usually only for the displayed records, which should be fine.


>> * I tried to make the most surgical change possible, which means I did not audit all the code for all uses of bytes, but only particularly the structured results and reports that used a NumberFormat or DecimalFormat and switched those to ByteFormat (where applicable).


I think this is fine. If we spot afterwards some places which also need to be adapted we could fix them.


>> * All JUnit tests passed - is there anything else that needs to be done before committing (in addition to any feedback)?


I think this is enough. Just let’s discuss a bit more on the couple of remarks about the Smart numbers.

Krum


From:
 mat-dev-bounces@xxxxxxxxxxx [mailto:mat-dev-bounces@xxxxxxxxxxx] On Behalf Of Kevin Grigorenko
Sent:
 Donnerstag, 26. Juni 2014 15:57
To:
 Memory Analyzer Dev list
Subject:
 Re: [mat-dev] Proposal: Add option to display bytes in KB, MB, GB, or Smart formats
 

Thanks! Take your time

--
Kevin Grigorenko
IBM WebSphere Foundation SWAT Team

kevin.grigorenko@xxxxxxxxxx
Blog:
https://www.ibm.com/developerworks/mydeveloperworks/blogs/kevgrig/?lang=en


Inactive hide details for "Tsvetkov, Krum" ---06/26/2014 05:30:03 AM---Hi Kevin, Sorry for the late reply."Tsvetkov, Krum" ---06/26/2014 05:30:03 AM---Hi Kevin, Sorry for the late reply.   


    From:
     

    To:
     

    Date:
     

    06/26/2014 05:30 AM
     

    Subject:
     

    Re: [mat-dev] Proposal: Add option to display bytes in KB, MB, GB, or Smart formats
     

    Sent by:
     






Hi Kevin,

Sorry for the late reply.
I like the idea to offer the possibility to display the sizes in different units.

I am currently looking into the patch. I may need some more time as it is pretty big. I hope afterwards I could answer your questions/concerns.

Krum



From:
 mat-dev-bounces@xxxxxxxxxxx [mailto:mat-dev-bounces@xxxxxxxxxxx] On Behalf Of Kevin Grigorenko
Sent:
 Dienstag, 17. Juni 2014 22:35
To:
 mat-dev@xxxxxxxxxxx
Subject:
 [mat-dev] Proposal: Add option to display bytes in KB, MB, GB, or Smart formats
 

Proposed patch to display bytes in KB, MB, GB, or Smart formats: https://bugs.eclipse.org/bugs/show_bug.cgi?id=437626 Diff: https://bugs.eclipse.org/bugs/attachment.cgi?id=244315&action="">

A few specific things I wanted to raise on the dev list:
* The Bytes class is added as a logical wrapper around a number of bytes. The Column class maps this to the BytesFormat formatter.
* The default option is BytesDisplay.Smart which changes the default MAT display behavior (which would be BytesDisplay.Bytes).
* The additional instances of the Bytes class might add some performance overhead.
* I tried to make the most surgical change possible, which means I did not audit all the code for all uses of bytes, but only particularly the structured results and reports that used a NumberFormat or DecimalFormat and switched those to ByteFormat (where applicable).
* All JUnit tests passed - is there anything else that needs to be done before committing (in addition to any feedback)?

--
Kevin Grigorenko
IBM WebSphere Foundation 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_______________________________________________
mat-dev mailing list
mat-dev@xxxxxxxxxxx

https://dev.eclipse.org/mailman/listinfo/mat-dev_______________________________________________
mat-dev mailing list
mat-dev@xxxxxxxxxxx

https://dev.eclipse.org/mailman/listinfo/mat-dev_______________________________________________
mat-dev mailing list
mat-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit

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
_______________________________________________
mat-dev mailing list
mat-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit

https://dev.eclipse.org/mailman/listinfo/mat-dev _______________________________________________
mat-dev mailing list
mat-dev@xxxxxxxxxxx
To change your delivery options, retrieve your password, or unsubscribe from this list, visit
https://dev.eclipse.org/mailman/listinfo/mat-dev

GIF image

GIF image

GIF image

GIF image


Back to the top