More reliable compressed-oops patch for JOL and some simple improvements [Patch Linked]

Aleksey Shipilev aleksey.shipilev at oracle.com
Tue Sep 9 15:48:43 UTC 2014


Hi Serkan,

On 09/08/2014 10:20 PM, serkan özal wrote:
> Patch Link: https://raw.githubusercontent.com/serkan-ozal/dropbox/master/compressed-oops.patch 		 	   		  

Thanks for doing this! I think we would need a few rounds of code
reviews to see what is going on there.

Comments:

 * I think the code is in a dire need of "Implementation notes" section
explaining what it tries to do. Previous code was also scarce on that,
but with this change, it is really over the top.

 * Are we sure the magic offsets in HotspotJvmClassAddressFinder are
stable across JDKs? I would not be that sure, but let's talk about that
later.

 * Do we need to have the AbstractHotspotJvmClassAddressFinder
hierarchy? It seems you can just turn the Abstract* class into the
concrete one, and accept the offsets there.

 * JvmInfo and JavaVersionInfo enums feel redundant. Would you like to
just make JAVA*.equals() calls in the getters like isJava_6()?

 * Please use shorter identifiers. Identifiers like
CLASS_DEFINITION_POINTER_OFFSET_IN_CLASS_INSTANCE_64_BIT_WITHOUT_COMPRESSED_REF_FOR_JAVA_8
are actually hard to read. CLASSDEF_CI_OFFSET_64_JAVA8 seems better.
This applies to field/local names as well.

 * We use 4 spaces as the indentation units, not tabs. Please also
auto-format the source, you have the indentation gone wrong a few times.

 * Do we actually need these two guys?
        private static final byte ADDRESS_SIZE_4_BYTE = 4;
        private static final byte ADDRESS_SIZE_8_BYTE = 8;

 * "////////////////////////" is not a good delimiter in our codebase.
Use "/* ------------------------- */"?

Thanks,
-Aleksey.



More information about the jol-dev mailing list