Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new DumpAllOffHeapEntriesCommand in DDR #21047

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LinHu2016
Copy link
Contributor

@LinHu2016 LinHu2016 commented Jan 31, 2025

  • new DumpAllOffHeapEntriesCommand provides a way to list all
    off-heap entries in the core file, which contain off-heap data
    (only when off-heap has been enabled in jvm and gcpolicy has been set
    to Balanced GC mode).
  • for the rest of cases(off-heap has been disabled or "old core file",
    which did not contain off-heap option at all), print out error message
    "This command only works with the core file,
    which contains off-heap."
  • off-heap entry information includes
    the address of the Array Object,
    the data address and
    the size of array data

Signed-off-by: lhu [email protected]

@LinHu2016 LinHu2016 force-pushed the DDR_update4offheap branch 2 times, most recently from 73c3599 to 37ff513 Compare February 3, 2025 16:20
@LinHu2016
Copy link
Contributor Author

LinHu2016 commented Feb 3, 2025

output format for DumpAllSparseHeapEntriesCommand

> !dumpalloffheapentries
List off-heap entries(521 entries)
+------------------+------------------+------------------
|  array object    |  data address    |      size        
+------------------+------------------+------------------
 0x00007f0889e2ff60 0x00007f025eef2000 0x0000000000014000
 0x00007f08b7aa1090 0x00007f025edc1000 0x0000000000020000
 0x00007f08b7aa1250 0x00007f025ede1000 0x0000000000020000
 0x00007f08b7aa1410 0x00007f025ee01000 0x0000000000020000
 0x00007f08b7aa15d0 0x00007f025ee21000 0x0000000000020000
 0x00007f08b7aa1790 0x00007f025ee41000 0x0000000000020000
 0x00007f08b7aa1950 0x00007f025ee61000 0x0000000000020000
 0x00007f088e09fc40 0x00007f025ee81000 0x0000000000021000
 0x00007f088cfa0190 0x00007f025eea2000 0x0000000000017000
 0x00007f088d34f5a8 0x00007f025eeb9000 0x0000000000021000
 0x00007f088c3febf0 0x00007f025eeda000 0x0000000000018000
...
 0x00007f08715cb6e8 0x00007f025798b000 0x0000000000010000
 0x00007f0869f8a8f8 0x00007f0257c6b000 0x0000000000010000
+------------------+------------------+------------------

@dmitripivkine
Copy link
Contributor

just curious, how this output looks like for Compressed Refs? I guess there are leading zeroes added to the object address.
BTW this PR still have a few whitespace issues (see console output for Line Endings Check job)

@LinHu2016 LinHu2016 force-pushed the DDR_update4offheap branch 2 times, most recently from 4658ac3 to c21c507 Compare February 3, 2025 18:56
@LinHu2016
Copy link
Contributor Author

just curious, how this output looks like for Compressed Refs? I guess there are leading zeroes added to the object address. BTW this PR still have a few whitespace issues (see console output for Line Endings Check job)

I think it always output 64 bit address for object pointer, and I will update for whitespace issues, thanks

@amicic amicic added the comp:gc label Feb 5, 2025
@amicic
Copy link
Contributor

amicic commented Feb 5, 2025

@keithc-ca please review

debugtools/DDR_VM/src/com/ibm/j9ddr/AuxFieldInfo29.dat Outdated Show resolved Hide resolved
Comment on lines 30 to 34
/**
* ObjectToSparseDataHashTable is used for accessing hash tables in SparseDataTableEntry
* (e.g. readAccessHashTable)
*/

public class ObjectToSparseDataHashTable extends HashTable_V1<MM_SparseDataTableEntryPointer>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please improve the javadoc (at least finish the sentence with a period) and remove the blank line separating it from the class.

Comment on lines 47 to 51
* @param structure
* the J9HashTablePointer
*
* @throws CorruptDataException
* when fails to read from structure
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tabs should only be used for indentation, please use spaces on lines 48, 51.

Comment on lines 65 to 72
public UDATA hash(MM_SparseDataTableEntryPointer entry) throws CorruptDataException {
return UDATA.cast(entry._dataPtr());
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please format consistently; the opening brace { should be on a separate line.

Comment on lines 37 to 41
protected ObjectToSparseDataHashTable(J9HashTablePointer hashTablePointer, boolean isInline, Class<MM_SparseDataTableEntryPointer> structType,
HashEqualFunction<MM_SparseDataTableEntryPointer> equalFn,
HashFunction<MM_SparseDataTableEntryPointer> hashFn) throws CorruptDataException
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please fold this, one parameter per line.

Comment on lines 24 to 28
public class DumpAllSparseHeapEntriesCommand extends Command
{
public DumpAllSparseHeapEntriesCommand()
{
addCommand("dumpalloffheapentries", "cmd|help", "dump all off heap entries");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest the class name should more closely match the command name.

addCommand("dumpalloffheapentries", "cmd|help", "dump all off heap entries");
}

public void run(String command, String[] args, Context context, PrintStream out) throws DDRInteractiveCommandException
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the missing @Override annotation.

if (0 != args.length) {
String argument = args[0];

if(argument.equalsIgnoreCase("help")) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Formatting: space before (.

HashTable<MM_SparseDataTableEntryPointer> readObjectToSparseDataTable = ObjectToSparseDataHashTable.fromJ9HashTable(objectToSparseDataTable);
SlotIterator<MM_SparseDataTableEntryPointer> readSlotIterator = readObjectToSparseDataTable.iterator();
long count = readObjectToSparseDataTable.getCount();
out.printf("List off heap entries(%,d entries)\n", count);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please %n, not \n in format strings in calls to printf() or format().

out.printf("List off heap entries(%,d entries)\n", count);
if (0 < count) {
if (!J9BuildFlags.J9VM_ENV_DATA64) {
throw new CorruptDataException("off heap would not be supported in 32 bit platform.");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please hyphenate "32-bit", and improve wording:

									throw new CorruptDataException("off heap is not supported on 32-bit platforms");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On second thought, this command should only be added in GetCommandsTask for 64-bit core files.

@LinHu2016 LinHu2016 force-pushed the DDR_update4offheap branch 4 times, most recently from 9aa77cb to 9e709bf Compare February 7, 2025 17:34
@keithc-ca keithc-ca changed the title Add new DumpAllSparseHeapEntriesCommand in DDR Add new DumpAllOffHeapEntriesCommand in DDR Feb 11, 2025

/**
* ObjectToSparseDataHashTable is used for accessing hash tables {@link HashTable}
* in SparseDataTableEntry {@link MM_SparseDataTableEntryPointer} (e.g. iterator)。
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use a normal period to finish sentences (unicode 0x2e).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all of those are fixed; see line 47.

return new ObjectToSparseDataHashTable(structure, false, MM_SparseDataTableEntryPointer.class, new SparseDataHashEqualFn(), new SparseDataHashFn());
}

private static class SparseDataHashFn implements HashFunction<MM_SparseDataTableEntryPointer>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's still not final.

@@ -45,6 +46,7 @@
import com.ibm.j9ddr.vm29.tools.ddrinteractive.commands.DumpAllRegionsCommand;
import com.ibm.j9ddr.vm29.tools.ddrinteractive.commands.DumpAllRomClassLinearCommand;
import com.ibm.j9ddr.vm29.tools.ddrinteractive.commands.DumpAllSegmentsCommand;
import com.ibm.j9ddr.vm29.tools.ddrinteractive.commands.DumpAllOffHeapEntriesCommand;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep imports sorted.

Comment on lines 43 to 48


public class DumpAllOffHeapEntriesCommand extends Command
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One blank line at a time is sufficient. Please add the missing javadoc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The javadoc is still missing.

Comment on lines 78 to 80
if (!J9BuildFlags.J9VM_ENV_DATA64) {
throw new CorruptDataException("32-bit JVM should not contain any off heap.%n");
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove this dead code (and the related import).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The import is still present; please remove it.

@LinHu2016 LinHu2016 force-pushed the DDR_update4offheap branch 3 times, most recently from 9caf04b to 27bd42b Compare February 13, 2025 17:16
}

/**
* Opens J9HashTable from J9HashTablePointer。
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use an ASCII period (., not ).

Comment on lines 28 to 40
import com.ibm.j9ddr.tools.ddrinteractive.Context;
import com.ibm.j9ddr.tools.ddrinteractive.DDRInteractiveCommandException;
import com.ibm.j9ddr.tools.ddrinteractive.Command;
import com.ibm.j9ddr.vm29.j9.HashTable;
import com.ibm.j9ddr.vm29.j9.ObjectToSparseDataHashTable;
import com.ibm.j9ddr.vm29.j9.SlotIterator;
import com.ibm.j9ddr.vm29.j9.gc.GCBase;
import com.ibm.j9ddr.vm29.pointer.generated.J9HashTablePointer;
import com.ibm.j9ddr.vm29.pointer.generated.MM_GCExtensionsPointer;
import com.ibm.j9ddr.vm29.pointer.generated.MM_SparseAddressOrderedFixedSizeDataPoolPointer;
import com.ibm.j9ddr.vm29.pointer.generated.MM_SparseDataTableEntryPointer;
import com.ibm.j9ddr.vm29.pointer.generated.MM_SparseVirtualMemoryPointer;
import com.ibm.j9ddr.vm29.pointer.VoidPointer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please sort imports.

{
public DumpAllOffHeapEntriesCommand()
{
addCommand("dumpalloffheapentries", "cmd|help", "dump all off-heap entries");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest cmd|help should instead be [help].

 - new DumpAllOffHeapEntriesCommand provides a way to list all
  off-heap entries in the core file, which contain off heap data
  (only when off-heap has been enabled in jvm and gcpolicy has been set
  to Balanced GC mode).
 - for the rest of cases(off-heap has been disabled or "old core file",
  which did not contain off-heap option at all), print out error message
  "This command only works with the core file,
  which contains off-heap."
 - off-heap entry information includes
   the address of the Array Object,
   the data address and
   the size of array data

Signed-off-by: lhu <[email protected]>
Comment on lines +31 to +40
import com.ibm.j9ddr.vm29.j9.gc.GCBase;
import com.ibm.j9ddr.vm29.j9.HashTable;
import com.ibm.j9ddr.vm29.j9.ObjectToSparseDataHashTable;
import com.ibm.j9ddr.vm29.j9.SlotIterator;
import com.ibm.j9ddr.vm29.pointer.generated.J9HashTablePointer;
import com.ibm.j9ddr.vm29.pointer.generated.MM_GCExtensionsPointer;
import com.ibm.j9ddr.vm29.pointer.generated.MM_SparseAddressOrderedFixedSizeDataPoolPointer;
import com.ibm.j9ddr.vm29.pointer.generated.MM_SparseDataTableEntryPointer;
import com.ibm.j9ddr.vm29.pointer.generated.MM_SparseVirtualMemoryPointer;
import com.ibm.j9ddr.vm29.pointer.VoidPointer;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Imports should be sorted with subpackages following their parent package; GCBase and VoidPointer should be placed like this:

import com.ibm.j9ddr.vm29.j9.SlotIterator;
import com.ibm.j9ddr.vm29.j9.gc.GCBase;
import com.ibm.j9ddr.vm29.pointer.VoidPointer;
import com.ibm.j9ddr.vm29.pointer.generated.J9HashTablePointer;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants