Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [mat-dev] Reworked collection extraction API

On Tue, 2014-10-21 at 13:36 +0100, Andrew Johnson wrote:
> That sounds quite a good idea - the difficulty was that the snapshot API
> is fairly general, so should apply to most VMs, but class library 
> internals
> varied from VM to VM and between versions,so making an API that works for
> all VMs is harder, and making one which is extensible by adopters for new
> VMs a bit harder still. Some of the collection classes have complex
> internals and I couldn't get quite all them all to work. Getting them to
> work for IBM Portable Heap Dump files which don't have field names
> is near impossible for some collections.

The class library differences and PHD files missing useful data are
still problems. My EE stuff has the same problem for EE implementation
details too.

The approach I've taken for class libraries is similar to the existing
code, where it has a big list of class name and JDK versions it knows
about, and has collection-extraction implementations for. That interface
is public so other code can both access collection information and
provide extraction implementations. It would probably make sense for
that to be an extension point, once it's gotten to an acceptable state
for public use.

The CollectionExtractor interface has method pairs like "boolean
hasCapacity()" and "Integer getCapacity(IObject)", repeated for other
properties. The first returns false if the property does not make sense
for the collection. If we can't determine the value (e.g. due to PHD
limitations) the latter returns null.

Doing that handles the differences well enough to pass the current test
suite and what I want to use for the EE queries.


I've stolen most the implementation logic from what is already there and
just re-organised it. If there are collection it doesn't handle
properly, that could just be bug/enhancement to be filed in bugzilla.

If it gets to the point where exposing it as public API is acceptable,
downstream products could just add an extension to declare their
additional extraction implementations.


I just posted a prototype patch plus a description to
https://bugs.eclipse.org/bugs/show_bug.cgi?id=442219. It's pretty hefty
(+3000/-2000 LOC), so I know will take a while to look over, but I think
is a start on a more widely usable collection-extraction API.


> If the Java EE queries were part of MAT in the API project then they could
> use internal interfaces before the interfaces were fully standardized.

I don't mind either way on whether it goes in the API project or a
separate one. If we wanted to keep a new collections API private for a
while, the API project may work better.


> One question is how wrapped collections should be reported. Both the
> inside collection and the wrapper could be flagged, but fixing the problem
> only saves one lot of space.

Currently I just have them both reported as valid collections. It would
be trivial to add an isWrapper() method so queries could distinguish if
they cared.


> Yes, you can run them from inside of Eclipse. See this, and if it isn't 
> clear then we should edit the wiki.
> http://wiki.eclipse.org/index.php?title=MemoryAnalyzer/Contributor_Reference#JUnit_Tests

Ah, I missed that I needed to use "JUnit Plugin Test" not "JUnit Test",
thanks. Doing the latter runs the "AllTests" suite as a single one and
you don't get per-test reporting or the ability to run a single one.


> > While testing, I've also noticed some inconsistencies in how the
> > existing code behaves for various collections. For example the fill
> > ratio of a zero-length array and zero-length list are not the same (0.0
> > vs 1.0). Is it important to keep the current behaviour for things like
> > that?
> > 
> The main reason for behaviour fill ratio zero-length objects is whether
> it should generate a finding in the component report etc. They are both
> full and empty! If the object is unresizable then generating a warning
> might make sense as a singleton could be used instead, but if it
> can grow then a zero sized collection might be needed and not be a
> problem.
> 
> The real warning should be in bytes of wasted space per collection rather
> than a percentage, but we don't currently calculate that.

That makes sense. I could add a Boolean isResizable flag to my API in
addition to an isWrapper one if that would make more logical sense.

-- 
James "Doc" Livingston
JBoss Support Engineering Group
Red Hat



Back to the top