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

[Lindenmaier, Goetz]

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.
>
>