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