RFR(L): 8064457: Introduce compressed oops mode "disjoint base" and improve compressed heap handling.

Tue Dec 2 23:46:03 UTC 2014

This looks good to me. Someone in runtime/gc have to look on it too.

universe.cpp about SystemProperty("com.sap.vm.test.compressedOopsMode"

we have:

java.vm.info=mixed mode, sharing

so we can have:

java.vm.compressedOopsMode=...

I am not expert in properties names but I don't want to have 'com.sap' 
in VM's property name.

virtualspace.cpp:

Could you fix release() - it does not reset _alignment?

In try_reserve_heap(), please, use (base == NULL) instead of (!base). 
And you don't need 'return;' in alignment check at the end of method.

In initialize_compressed_heap() again (!_base). You don't stop (check 
(base == NULL)) after successful unscaled, zerobased, disjointbase 
allocations. You need to separate them with the check:

+
+    }
+  }
+  if (_base == NULL) {
+
+    if (PrintCompressedOopsMode && Verbose) {
+      tty->print(" == Z E R O B A S E D ==\n");
+    }

and so on.

num_attempts calculation and while() loop are similar in unscaled and 
zerobased cases. Could you move it into a separate method?

In disjointbase while() condition no need for _base second check:

+           (_base == NULL ||
+            ((_base + size > (char *)OopEncodingHeapMax) &&

Thanks,
Vladimir

On 11/21/14 5:31 AM, Lindenmaier, Goetz wrote:
> Hi,
>
> I prepared a new webrev trying to cover all the issues mentioned below.
> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.01/
>
> I moved functionality from os.cpp and universe.cpp into
> ReservedHeapSpace::initialize_compressed_heap().
> This class offers to save _base and _special, which I would have to reimplement
> if I had improved the methods I had added to os.cpp to also allocate large page
> heaps.
> Anyways, I think this class is the right place to gather code trying to reserve
> the heap.
> Also, I get along without setting the shift, base, implicit_null_check etc. fields
> of Universe, so there is no unnecessary calling back and forth between the two
> classes.
> Universe gets the heap back, and then sets the properties it needs to configure
> the compressed oops.
> All code handling the noaccess prefix is in a single method, too.
>
> Best regards,
>    Goetz.
>
> Btw, I had to workaround a SS12u1 problem: it wouldn't compile
> char * x = (char*)UnscaledOopHeapMax - size in 32-bit mode.
>
>
> -----Original Message-----
> From: hotspot-dev [mailto:hotspot-dev-bounces at openjdk.java.net] On Behalf Of Lindenmaier, Goetz
> Sent: Montag, 17. November 2014 09:33
> To: 'Vladimir Kozlov'; 'hotspot-dev at openjdk.java.net'
> Subject: RE: RFR(L): 8064457: Introduce compressed oops mode "disjoint base" and improve compressed heap handling.
>
> Hi Vladimir,
>
>> It is very significant rewriting and it takes time to evaluate it.
> Yes, I know ... and I don't want to push, but nevertheless a ping
> can be useful sometimes. Thanks a lot for looking at it.
>
>> And I would not say it is simpler then before :)
> If I fix what you propose it's gonna get even more simple ;)
>> These is what I found so far.
>
>> The idea to try to allocate in a range instead of just below
>> UnscaledOopHeapMax or OopEncodingHeapMax is good. So I would ask to do
>> several attempts (3?) on non_PPC64 platforms too.
> Set to 3.
>
>> It is matter of preference but I am not comfortable with switch in loop.
>> For me sequential 'if (addr == 0)' checks is simpler.
> I'll fix this.
>
>> One thing worries me that you release found space and try to get it
>> again with ReservedHeapSpace. Is it possible to add new
>> ReservedHeapSpace ctor which simple use already allocated space?
> This was to keep diff's small, but I also think a new constructor is good.
> I'll fix this.
>
>> The next code in ReservedHeapSpace() is hard to understand ():
>> (UseCompressedOops && (requested_address == NULL ||
> requested_address+size > (char*)OopEncodingHeapMax) ?
>> may be move all this into noaccess_prefix_size() and add comments.
> I have to redo this anyways if I make new constructors.
>
>> Why you need prefix when requested_address == NULL?
> If we allocate with NULL, we most probably will get a heap where
> base != NULL and thus need a noaccess prefix.
>
>> Remove next comment in universe.cpp:
>> // SAPJVM GL 2014-09-22
> Removed.
>
>> Again you will release space so why bother to include space for classes?:
>> +          // For small heaps, save some space for compressed class pointer
>> +          // space so it can be decoded with no base.
> This was done like this before.  We must assure the upper bound of the
> heap is low enough that the compressed class space still fits in there.
>
> virtualspace.cpp
>
>> With new code size+noaccess_prefix could be requested. But later it is
>> not used if WIN64_ONLY(&& UseLargePages) and you will have empty
>> non-protected page below heap.
> There's several points to this:
>   * Also if not protectable, the heap base has to be below the real start of the
>      heap.  Else the first object in the heap will be compressed to 'null'
>      and decompression will fail.
>   * If we don't reserve the memory other stuff can end up in this space. On
>      errors, if would be quite unexpected to find memory there.
>   * To get a heap for the new disjoint mode I must control the size of this.
>       Requesting a heap starting at (aligned base + prefix) is more likely to fail.
>   * The size for the prefix must anyways be considered when deciding whether the
>       heap is small enough to run with compressed oops.
> So distinguishing the case where we really can omit this would require
> quite some additional checks everywhere, and I thought it's not worth it.
>
> matcher.hpp
>
>> Universe::narrow_oop_use_implicit_null_checks() should be true for such
>> case too. So you can add new condition with || to existing ones. The
>> only condition you relax is base != NULL. Right?
> Yes, that's how it's intended.
>
> arguments.* files
>
>> Why you need PropertyList_add changes.
> Oh, the code using it got lost.  I commented on this in the description in the webrev.
> "To more efficiently run expensive tests in various compressed oop modes, we set a property with the mode the VM is running in. So far it's called "com.sap.vm.test.compressedOopsMode" better suggestions are welcome (and necessary I guess). Our long running tests that are supposed to run in a dedicated compressed oop mode check this property and abort themselves if it's not the expected mode."
> When I know about the heap I do
>      Arguments::PropertyList_add(new SystemProperty("com.sap.vm.test.compressedOopsMode",
>                                                     narrow_oop_mode_to_string(narrow_oop_mode()),
>                                                     false));
> in universe.cpp.
> On some OSes it's deterministic which modes work, there we don't start such tests.
> Others, as you mentioned OSX,  are very indeterministic.  Here we save testruntime with this.
> But it's not that important.
> We can still parse the PrintCompresseOopsMode output after the test and discard the
> run.
>
>> Do you have platform specific changes?
> Yes, for ppc and aix.  I'll submit them once this is in.
>
>  From your other mail:
>> One more thing. You should allow an allocation in the range when returned from OS allocated address does not match
>> requested address. We had such cases on OSX, for example, when OS allocates at different address but still inside range.
> Good point.  I'll fix that in os::attempt_reserve_memory_in_range.
>
> I'll ping again once a new webrev is done!
>
> Best regards,
>    Goetz.
>
>
> On 11/10/14 6:57 AM, Lindenmaier, Goetz wrote:
>> Hi,
>>
>> I need to improve a row of things around compressed oops heap handling
>> to achieve good performance on ppc.
>> I prepared a first webrev for review:
>> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.00/
>>
>> A detailed technical description of the change is in the webrev and according bug.
>>
>> If requested, I will split the change into parts with more respective less impact on
>> non-ppc platforms.
>>
>> The change is derived from well-tested code in our VM.  Originally it was
>> crafted to require the least changes of  VM coding, I changed it to be better
>> streamlined with the VM.
>> I tested this change to deliver heaps at about the same addresses as before.
>> Heap addresses mostly differ in lower bits.  In some cases (Solaris 5.11) a heap
>> in a better compressed oops mode is found, though.
>> I ran (and adapted) test/runtime/CompressedOops and gc/arguments/TestUseCompressedOops*.
>>
>> Best regards,
>>     Goetz.
>>
>>