jifa icon indicating copy to clipboard operation
jifa copied to clipboard

List objects by outgoing references differs from MAT behaviour

Open jasonk000 opened this issue 3 years ago • 6 comments

The behaviour is different between MAT and JIFA, which leads to some confusion for our users.

In MAT, when I click an object, I can choose:

  • By object, or By Class, and List Outgoing or List Incoming

In JIFA, I can do the same: image

However, the behaviour is slightly different.

Reproduction steps:

  • In JIFA and MAT, both, .. open a heapdump, and go to Histogram
  • Choose a class instance, right click
  • In JIFA, choose References By Object -> Outgoing References
  • In MAT, choose List Objects -> with Outgoing References

In JIFA, you see the class entry, with outgoing references from the class

image

In MAT, you see a list of all instances, with outgoing references from each instance image

jasonk000 avatar Jul 27 '22 19:07 jasonk000

The difference could be when a IContextObjectSet is available rather than just an IContextObject.

IContextObject IContextObjectSet

The IContextObjectSet has a getObjectId() like IContextObject which when available is used to set the object in the object inspector. This would be the class object in the histogram. The IContextObjectSet also has getObjectIds() which for the histogram returns all the instances of the class. getOQL() returns the OQL for those objects, if available. That is used for the 'Copy OQL' query, and also for the special case when the underlying object has not been indexed (because it has been discarded) but a reference to that object has been obtained. The OQL will refer to the object by address rather than ID.

ajohnson1 avatar Aug 30 '22 14:08 ajohnson1

If I'm not mistaken, I think the histogram comes through JIFA from here:

return PageViewBuilder.<ClassHistogramRecord, Model.Histogram.Item>fromList(records)
    .beforeMap(record -> $(() -> record
    .calculateRetainedSize(context.snapshot, true, true, Helper.VOID_LISTENER)))
    .paging(new PagingRequest(page, pageSize))
    .map(record -> new Model.Histogram.Item(record.getClassId(), record.getLabel(),
                                            Model.Histogram.ItemType.CLASS_LOADER,
                                            record.getNumberOfObjects(),
                                            record.getUsedHeapSize(),
                                            record.getRetainedHeapSize()))

Here we pass the getClassId() into the Histogram.Item constructor:

    public Item(int objectId, String label, int type, long numberOfObjects, long shallowSize,

And we set the context menu target to the objectId:

    <span v-if="scope.row.isRecord"
        @click="scope.row.type!==6?$emit('setSelectedObjectId', scope.row.objectId):{}"
        @contextmenu="contextMenuTargetObjectId = scope.row.objectId; contextMenuTargetObjectLabel = scope.row.label"
        v-contextmenu:contextmenu
        style="cursor: pointer">

== Instead, what I think needs to happen is the context target object Id, should be a list and contain all the object IDs that were found and then can be passed through, or we introduce some more logic like this, where it dynamically fetches the objects into a new tab..

Thoughts?

jasonk000 avatar Sep 04 '22 01:09 jasonk000

Thanks for the comments.

I think Jifa should obey the same rule and also provide the same(as possible) user interface as MAT. Are you working on this now? If not, I will try to fix it once I have time.

D-D-H avatar Sep 05 '22 05:09 D-D-H

I am not currently working on it, and can't see myself having time before Oct earliest.

jasonk000 avatar Sep 05 '22 05:09 jasonk000

I am not currently working on it, and can't see myself having time before Oct earliest.

Okay, I will try to fix it.

D-D-H avatar Sep 05 '22 05:09 D-D-H

@jasonk000

I made a quick fix for this problem.

https://github.com/eclipse/jifa/pull/179

But this patch only fixes the problem when group_by = by_class. For other group_by values, there is another problem, we need to fix them first:-(

If it works for you, please help verify & review it.

D-D-H avatar Sep 06 '22 07:09 D-D-H