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

Lindenmaier, Goetz goetz.lindenmaier at sap.com
Tue Jan 6 07:56:22 UTC 2015


Hi Coleen,

thanks a lot for pushing this change!

Best regards,
  Goetz.

-----Original Message-----
From: Coleen Phillimore [mailto:coleen.phillimore at oracle.com] 
Sent: Monday, January 05, 2015 6:01 PM
To: Lindenmaier, Goetz; hotspot-runtime-dev at openjdk.java.net; 'hotspot-dev at openjdk.java.net'
Subject: Re: RFR(L): 8064457: Introduce compressed oops mode "disjoint base" and improve compressed heap handling.


On 1/5/15, 11:49 AM, Coleen Phillimore wrote:
>
> On 1/5/15, 4:24 AM, Lindenmaier, Goetz wrote:
>> Hi Coleen,
>>
>> It would be great if you could sponsor this change.
>
> I see no other comments so I'll push it today.

I am planning to push this to hs-rt, btw.
Coleen

>>
>>> The metaspace case probably can be fixed to use the 
>>> try_reserve_heap() code maybe.
>> We wondered why the compressed class space is always above the heap.
>> If the heap is too big for unscaled, the compressed class space could 
>> be placed
>> in the lower 4G.
>
> The reason that we chose the compressed class space on the top of the 
> heap is for a couple reasons.  The time spent compressing and 
> decompressing oops is more critical than the time spent doing the same 
> for the _klass pointer, so we wanted to make the system more likely to 
> get unscaled compressed oops, or even zero based compressed oops.
>
> On solaris, the space below the heap is limited because by default 
> malloc uses it so we were limited in low address memory that we can 
> use.  We want the compressed class space code not be platform 
> dependent.  We also need to have the CDS archive at a fixed address 
> near or adjacent to the compressed class space, so were running out at 
> the bottom of the heap.
>
> The compressed class space is fixed at startup (just like PermGen was) 
> and the classes are variable sized, so we had to pick a size fairly 
> large so that it's unlikely for applications to run out. If above 32G, 
> we might be able to implement code to allocate a new space which would 
> use the same compression algorithm.  We haven't had a request for that 
> but it's something we could do.
>
>
>> The aarch port brings some changes here, and we would like to 
>> investigate
>> this on top of the aarch changes, if that's fine with you.
>
> So one thing I prototyped and really wanted to check in was what we 
> called indirect class pointers.  Instead of allocating classes in a 
> fixed class space, we allocate a fixed array of Klass pointers and the 
> objects in the heap point to this.   It had a 8% performance 
> degradation in specbb2008 compiler.compiler (iirc, or something like 
> that).
>
> This solved so many problems though.  If you investigated this sort of 
> approach, I'd be very grateful.   I haven't had time to go back to 
> figure out why this had this degradation (we thought it was Java 
> vtable accesses).
>
> Thanks,
> Coleen
>>
>> Thanks a lot for going through this complex review!
>>
>> Best regards,
>>    Goetz.
>>
>> -----Original Message-----
>> From: Coleen Phillimore [mailto:coleen.phillimore at oracle.com]
>> Sent: Freitag, 2. Januar 2015 17:27
>> To: Lindenmaier, Goetz; hotspot-runtime-dev at openjdk.java.net
>> Subject: Re: RFR(L): 8064457: Introduce compressed oops mode 
>> "disjoint base" and improve compressed heap handling.
>>
>>
>> On 1/2/15, 10:09 AM, Lindenmaier, Goetz wrote:
>>> Hi Coleen,
>>>
>>> I looked at the cleanups and found both not straight forward to 
>>> implement.
>>>
>>> 1) Merging ReservedHeap::initialize and 
>>> ReservedHeapSpace::try_reserve_heap.
>>>    The new code calls initialize() with requested_address = NULL, 
>>> but metaspace.cpp
>>> passes a real requested_address.  Therefore I can not clean up the 
>>> calls to
>>> failed_to_reserve_as_requested() in initialize().  Without that 
>>> cleanup, I need to pass flags
>>> into initialize() to configure all the different behaviours I would 
>>> need in a
>>> merged case.
>>> I think with 'large' and 'requested_address' there is already enough 
>>> cases
>>> in that method, so that I don't want to add more.
>> Okay, this is fine.  More flags to special case functions aren't good
>> either.  Can you file an RFE to look into refactoring so that the
>> duplicate code can be removed though?   The metaspace case probably can
>> be fixed to use the try_reserve_heap() code maybe.
>>
>>> 2) Moving _noaccess_prefix to ReservedHeapSpace
>>>
>>> This does not work as  ReservedHeapSpace is casted to
>>> ReservedSpace and returned by value.  So ReservedHeapSpace can not have
>>> fields or virtual methods.
>>> It would be probably better to change the code to return 
>>> ReservedHeapSpace
>>> by pointer, or, at least under that type, but I think that exceeds 
>>> the scope of this change.
>> Too bad.
>>
>>> So is it ok if I leave the change as is wrt. to these cleanups?
>> Yes, this is fine.
>>> I extended test/runtime/CompressedOops/UseCompressedOops.java
>>> to combine the testcases of the different mode with the GCs and 
>>> UseLargePages.
>>> I added the changes in the test to
>>> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.03/
>> Is .03 with the additional changes?  If so, you should have made an .04.
>>
>> I reviewed this again and it looks good.   I think Vladimir has reviewed
>> an earlier version.  I can sponsor this assuming there are no more 
>> comments.
>>
>> Coleen
>>> Best regards,
>>>     Goetz.
>>>
>>>
>>>
>>>
>>>
>>> -----Original Message-----
>>> From: Coleen Phillimore [mailto:coleen.phillimore at oracle.com]
>>> Sent: Dienstag, 30. Dezember 2014 16:49
>>> To: Lindenmaier, Goetz; hotspot-runtime-dev at openjdk.java.net
>>> Subject: Re: RFR(L): 8064457: Introduce compressed oops mode 
>>> "disjoint base" and improve compressed heap handling.
>>>
>>>
>>> On 12/30/14, 8:42 AM, Lindenmaier, Goetz wrote:
>>>> Hi Coleen,
>>>>
>>>>> disjoint
>>>> It tries to say that the bits used for the base (36-63) are 
>>>> different from
>>>> those used for the oops / used as offset into the heap (0-35).
>>>> I chose the name according to this translation
>>>> http://dict.leo.org/#/search=disjoint&searchLoc=0&resultOrder=basic&multiwordShowSingle=on 
>>>>
>>>> Both the german adjective and verb fit to describe the property of 
>>>> the base.
>>>> I can add the following to the docu of the mode in universe.hpp:358
>>>>      //     Disjoint: Bits used in base are disjoint from bits used
>>>>      //     for oops ==> oop = (cOop << 3) | base. One can disjoint
>>>>     //     the bits of an oop into base and compressed oop.
>>> Yes, that helps.  It's a good distinct word so its good that we can 
>>> make
>>> sense in the documentation.
>>>>> (I filed a bug to move these to the memory directory)
>>>> Good idea!
>>>>
>>>>>      272 void ReservedSpace::establish_noaccess_prefix() {
>>>>> This function should be in ReservedHeapSpace
>>>> Yes, I moved it.  I also would like to move the field 
>>>> _noaccess_prefix,
>>>> but I would have to change a lot of code, like 
>>>> ReservedSpace::release().
>>>> The problem is that _base is not the real base in ReservedHeapSpace.
>>>> Probably ReservedHeapSpace should have heap_size() and heap_base()
>>>> giving the correcte values.
>>>> I'll try to come up with a patch on top of this, maybe we should do it
>>>> in a separate bug.
>>> I just moved _noaccess_prefix and added a virtual
>>> ReservedHeapSpace::release() that adjusted the base and size with
>>> noaccess_prefix.  I don't think the footprint of a vtable or virtual
>>> call makes any difference.
>>>>> Can you have arguments.cpp verify_range of HeapSearchSteps so this 
>>>>> can't
>>>>> happen?
>>>> Done.
>>>>
>>>>> Rather move this to the end of ReservedHeapSpace constructor.
>>>>> assert(_base == NULL || ...
>>>> Yes, you are right, it should not be there twice.  I moved it into the
>>>> constructor.  It is only relevant for heaps, right?
>>> Yes.
>>>>> Code in try_reserve_heap() is mostly a copy of  
>>>>> ReservedSpace::initialize()
>>>> You are right,  requested_address in initialize is dead now.  I'll 
>>>> remove that
>>>> and try to merge the two functions.
>>>>
>>>> I removed the verbose printing.  Anyways, it's mostly useful when 
>>>> developing this.
>>> Thanks.  I was trying to find some way to move the printing so it would
>>> make sense to me but ended up giving up on that.
>>>>> aligned_HBMA
>>>> Changed to aligned_heap_base_min_address.
>>>>
>>>>> Is "size" const in initialize_compressed_heap?
>>>> Yes.  Added 'const'.
>>>>
>>>> I'll run tests with +UseLargePages.
>>>>
>>>> I had verified that the heap is at the exact same position for the 
>>>> code in
>>>> my first RFR, and with HeapSearchSteps=1 to make sure I don't break 
>>>> anything.
>>>> Vladimir asked me to increase that to 3.  So far I still didn't 
>>>> find a case where
>>>> this changed anything.
>>>> (But I know from SAP JVM, which is tested on all the OS variants we 
>>>> support,
>>>> that there are cases where this can lead to a better - but then 
>>>> different - heap
>>>> placement.)
>>>> The only case where there is always a difference is Solaris 5.11 which
>>>> now can allocate zerobased memory.
>>>> The code passes the CDS jtreg tests.
>>>>
>>>> I updated the webrev.03 with the minor changes.
>>>> I'll come up with a new one with the cleanups of Reserved(Heap)Space
>>>> (_noaccess_prefix and redundant functions).
>>>>
>>>> Thanks for this thorough review!!
>>> Thank you for your willingness to make these changes.
>>>
>>> Happy New Year!
>>> Coleen
>>>
>>>> Happy new year,
>>>>      Goetz.
>>>>
>>>>
>>>>
>>>>
>>>> -----Original Message-----
>>>> From: hotspot-runtime-dev 
>>>> [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of 
>>>> Coleen Phillimore
>>>> Sent: Dienstag, 30. Dezember 2014 03:06
>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>> Subject: Re: RFR(L): 8064457: Introduce compressed oops mode 
>>>> "disjoint base" and improve compressed heap handling.
>>>>
>>>>
>>>> Hi,  I've reviewed more code.
>>>>
>>>> I'm sorry about the following comment that should have been asked
>>>> before.  The name of the mode is very confusing. I can't figure out 
>>>> why
>>>> it's called "disjoint".  I keep looking for a second heap base or some
>>>> code that makes the heap discontinuous.   The heap base is really
>>>> specially aligned, right?  So I went to the thesaurus.com:
>>>>
>>>> http://www.thesaurus.com/browse/aligned
>>>>
>>>> and "disjoin" is an antonym.   Why was "disjoint" chosen? From the
>>>> code, it looks like "aligned" or "scaled" are more descriptive but
>>>> they're overused words in the code.  But ScaledBasedNarrowOop seems 
>>>> more
>>>> descriptive than Disjoint.
>>>>
>>>> In virtualspace.cpp (I filed a bug to move these to the memory 
>>>> directory)
>>>>
>>>>      272 void ReservedSpace::establish_noaccess_prefix() {
>>>>
>>>>
>>>> This function should be in ReservedHeapSpace since ReservedSpace never
>>>> does this (and we never want it to).  The function it replaced was in
>>>> ReservedSpace and that was wrong.  Now that ReservedHeapSpace has
>>>> compressed oops logic in it, I think this function should be moved to
>>>> this class.   Actually, I've modified your patch to move the
>>>> _noaccess_prefix field to ReservedHeapSpace.  Maybe that's too much
>>>> change to your patch but since the handling of noaccess_prefix is
>>>> changed, it seemed like the thing to do.
>>>>
>>>> Can you have arguments.cpp verify_range of HeapSearchSteps so this 
>>>> can't
>>>> happen?
>>>>
>>>>       guarantee(HeapSearchSteps > 0, "Don't set HeapSearchSteps to 
>>>> 0");
>>>>
>>>> Can this be a function like check_encoding_for_mark_sweep() , or
>>>> something like that?  It appears twice.  Rather move this to the 
>>>> end of
>>>> ReservedHeapSpace constructor.
>>>>
>>>>       assert(_base == NULL ||
>>>> markOopDesc::encode_pointer_as_mark(_base)->decode_pointer() == _base,
>>>>              "area must be distinguishable from marks for 
>>>> mark-sweep");
>>>>       assert(_base == NULL ||
>>>> markOopDesc::encode_pointer_as_mark(&_base[size])->decode_pointer() ==
>>>> &_base[size],
>>>>              "area must be distinguishable from marks for 
>>>> mark-sweep");
>>>>
>>>> Code in try_reserve_heap() is a mostly a copy of
>>>> ReservedSpace::initialize(), except the first version has a call to
>>>> failed_to_reserve_as_requested which looks like it's handling the case
>>>> for not being able to reserve the requested address for compressed 
>>>> oops
>>>> which seems like you don't want this.  This is very confusing.  There
>>>> shouldn't be two of these.
>>>>
>>>> It looks like in the compressed oops case, you call
>>>> ReservedSpace::initialize() only with NULL requested address so
>>>> failed_to_reserve_as_requested is dead code with this change.  So it
>>>> seems like try_reserve_heap() could be ReservedSpace::initialize() 
>>>> with
>>>> the failed_to_reserve_as_requested eliminated and a special case for
>>>> handling unaligned addresses.
>>>>
>>>> The logic in initialize_compressed_heap() still doesn't make sense to
>>>> me.   If the first try_reserve_range() succeeds at getting an unscaled
>>>> compressed oop base, then does PrintCompressedOopsMode && Verbose 
>>>> still
>>>> print:
>>>>
>>>>           tty->print(" == U N S C A L E D ==\n");
>>>>           tty->print(" == Z E R O B A S E D ==\n");
>>>>           tty->print(" == D I S J O I N T B A S E ==\n");
>>>>           tty->print(" == H E A P B A S E D ==\n");
>>>>
>>>> Why would you print all 4?   It's the printing that makes this
>>>> confusing!  Now I see how the next case checks that the base 
>>>> address is
>>>> a better allocation.  If you changed the printing to a comment like:
>>>>
>>>> // UnscaledMode after testing previous base address is not better (or
>>>> something).
>>>>
>>>> Actually, you have these comments.  I think the printing should be
>>>> removed because it's the opposite of helpful.
>>>>
>>>> I had trouble reading the parameter aligned_HBMA, can you change to
>>>> aligned_heap_base_min (I know it's longer but there's too many
>>>> calculations here that words will make easier to read).  Or just
>>>> heap_base_min_address.
>>>>
>>>>           zerobased_max = (char *)OopEncodingHeapMax - class_space;
>>>>
>>>> Odd that the solaris compiler can compile this and not the code below.
>>>> (minor point) I think this should be:
>>>>
>>>>           zerobased_max = zerobased_max - class_space;
>>>>
>>>> Is "size" const in initialize_compressed_heap? (answer appears to be
>>>> yes)  It's useful for reading the calculations that it never 
>>>> changes so
>>>> I think it should be const (it's a minor point too).
>>>>
>>>> I don't have any other comments, I've tried to do the calculations 
>>>> a bit
>>>> but I know that you've done this yourself.
>>>>
>>>> Also, can you run tests with -XX:+UseLargePages?   I assume you've run
>>>> tests with the 4 garbage collectors.  This is a tricky piece of code
>>>> that replaced something else difficult.  We've had to change it 
>>>> often to
>>>> support other features like CDS and compressed class pointers so 
>>>> making
>>>> it as understandable as possible is worth the extra effort. I think
>>>> your new design looks cleaner.
>>>>
>>>> Thanks,
>>>> Coleen
>>>>
>>>>
>>>> On 12/24/14, 6:13 PM, Coleen Phillimore wrote:
>>>>> One note for my first reviewed file.
>>>>>
>>>>> On 12/22/14, 5:30 PM, Coleen Phillimore wrote:
>>>>>> Hi Goetz,
>>>>>>
>>>>>> I'm looking at this version of the code
>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.03/ 
>>>>>>
>>>>>>
>>>>>> This passes our JPRT tests.  Yeah!
>>>>>>
>>>>>> It's a significant change, so I'm actually reading the code with the
>>>>>> patches applied.
>>>>>>
>>>>>> In src/share/vm/memory/universe.cpp
>>>>>>
>>>>>> Can you put all this code from 742-776 in a new function above
>>>>>> initialize_heap(), like set_narrow_oop_values()?
>>>>>> The comment at 689-695 refer to code that's moved to
>>>>>> virtualspace.cpp.  I think it needs to be reworded, updated for
>>>>>> disjoint oops mode or removed. The comment at 744-749 also refers to
>>>>>> code that's moved so should be removed also.   The last two lines of
>>>>>> this comment refer to something whose implementation has changed, so
>>>>>> just remove them.
>>>>>>
>>>>>> On line
>>>>>>
>>>>>> 750 if ((uint64_t)Universe::heap()->reserved_region().end() >
>>>>>> UnscaledOopHeapMax) {
>>>>>>
>>>>>> Can you make this expression a variable because the same expression
>>>>>> appears on line 754.
>>>>>>
>>>>>> Why can't this code at 742-776 be with the compressed oops mode code
>>>>>> at line 836-839.  If you make this set_narrow_oop_values() function,
>>>>>> can it be moved together?
>>>>> Apparently this doesn't work (it crashes on some platforms).  I can't
>>>>> completely follow the control flow that makes this wrong, but ignore
>>>>> this suggestion.
>>>>>
>>>>> Coleen
>>>>>> Can you remove this comment too? I have no idea what it means. There
>>>>>> is no CATCH below (?)
>>>>>>
>>>>>>      // We will never reach the CATCH below since Exceptions::_throw
>>>>>> will cause
>>>>>>      // the VM to exit if an exception is thrown during 
>>>>>> initialization
>>>>>>
>>>>>>
>>>>>> ** We should file an RFE to move virtualspace.hpp/cpp to the
>>>>>> src/share/vm/memory directory!
>>>>>>
>>>>>> Sorry, this is partial, I'll continue tomorrow.
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>>>
>>>>>>
>>>>>> On 12/11/2014 06:18 AM, Lindenmaier, Goetz wrote:
>>>>>>> Hi Coleen,
>>>>>>>
>>>>>>> thanks for looking at this change!
>>>>>>>
>>>>>>>> get_attach_addresses_for_disjoint_mode function belongs in
>>>>>>>> virtualspace.cpp
>>>>>>> All information about the bounds of the modes comes from universe.
>>>>>>> This is
>>>>>>> related to is_disjoint_heap_base_address() and thus I left it in
>>>>>>> universe.
>>>>>>> But I'll move it, it can also be regarded as an implementation
>>>>>>> detail of
>>>>>>> initialize_compressed_heap().  I can make it static then, so that
>>>>>>> it's removed
>>>>>>> in 32 bit build, which is good.
>>>>>>>
>>>>>>>> Can you reduce some to at least 100?
>>>>>>> Done. I agree that's better . (I have a  2560x1440 screen, causing
>>>>>>> me to have
>>>>>>> big windows :))
>>>>>>>
>>>>>>>> Why is highest_start not const char* ?
>>>>>>> Removed the const.
>>>>>>>
>>>>>>>> The control flow in initialize_compressed_heap makes no sense 
>>>>>>>> to me.
>>>>>>> Vladimir had the same problem, I added comment at line 493 to make
>>>>>>> that clearer.
>>>>>>> One problem of the previous implementation was that, if the os 
>>>>>>> layer
>>>>>>> returned an address with desired properties, but not at the 
>>>>>>> requested
>>>>>>> address, it was discarded.
>>>>>>> To fix this, try_reserve_heap always returns what it allocated, 
>>>>>>> and I
>>>>>>> check afterwards.
>>>>>>> Therefore the checks of the next mode include the check of the
>>>>>>> previous one.  So if I try to allocate disjoint, but get unscaled,
>>>>>>> I will detect that and return the unscaled heap.
>>>>>>> If I implement a return, I just have to add tests.  I don't get 
>>>>>>> rid of
>>>>>>> other tests.
>>>>>>>
>>>>>>>> Also, our 32 bit platforms don't like: 1 * SIZE_64K * SIZE_32G,
>>>>>>> I changed that to use UCONST64 macro.  That should help I guess.
>>>>>>> I also have these in globalDefinitions_xlc.hpp. Should I move them
>>>>>>> to globalDefinitions.hpp:200, after definition of K, M and G?
>>>>>>>
>>>>>>> I uploaded a new webrev:
>>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.03/ 
>>>>>>>
>>>>>>>
>>>>>>> Best regards,
>>>>>>>       Goetz.
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> -----Original Message-----
>>>>>>> From: hotspot-runtime-dev
>>>>>>> [mailto:hotspot-runtime-dev-bounces at openjdk.java.net] On Behalf Of
>>>>>>> Coleen Phillimore
>>>>>>> Sent: Mittwoch, 10. Dezember 2014 20:22
>>>>>>> To: hotspot-runtime-dev at openjdk.java.net
>>>>>>> Subject: Re: RFR(L): 8064457: Introduce compressed oops mode
>>>>>>> "disjoint base" and improve compressed heap handling.
>>>>>>>
>>>>>>>
>>>>>>> Hi Goetz,
>>>>>>>
>>>>>>> I have some initial comments, which are much less detailed than
>>>>>>> Vladimir's comments.  I haven't actually processed all the
>>>>>>> implementation details yet.
>>>>>>>
>>>>>>> I think get_attach_addresses_for_disjoint_mode function belongs in
>>>>>>> virtualspace.cpp and not in universe.  I like that the 
>>>>>>> compressed oop
>>>>>>> reservation logic is moved to virtualspace.cpp.  I think this is an
>>>>>>> improvement over the Universe::preferred_heap_base() logic which 
>>>>>>> was
>>>>>>> also difficult.
>>>>>>>
>>>>>>> The Hotspot coding style doesn't dictate 80 columns anymore 
>>>>>>> (there is
>>>>>>> debate whether it should have or not, which we try not to have this
>>>>>>> debate), but I found the very long lines in this code 
>>>>>>> inconsistent with
>>>>>>> other Hotspot code.  I had to expand my window to cover my whole
>>>>>>> screen.   Can you reduce some to at least 100?
>>>>>>>
>>>>>>> For example, I aligned these parameters to see better since there
>>>>>>> are so
>>>>>>> many (the indentation isn't right in email).
>>>>>>>
>>>>>>> void ReservedHeapSpace::try_reserve_range(char *const 
>>>>>>> highest_start,
>>>>>>>                                                char *lowest_start,
>>>>>>>                                                size_t
>>>>>>> attach_point_alignment,
>>>>>>>                                                char 
>>>>>>> *aligned_HBMA,   //
>>>>>>> HeapBaseMinAddress
>>>>>>>                                                char *upper_bound,
>>>>>>>                                                size_t size,
>>>>>>>                                                size_t alignment,
>>>>>>>                                                bool large) {
>>>>>>>
>>>>>>> Why is highest_start not const char* ?  Doesn't char* const
>>>>>>> highest_start just restrict you from changing the pointer and 
>>>>>>> not what
>>>>>>> it points to?  This seems odd to do.
>>>>>>>
>>>>>>> The control flow in initialize_compressed_heap makes no sense to 
>>>>>>> me.
>>>>>>> It seems like there should be an exit when the best allocation for
>>>>>>> compression is achieved.   But it falls through after
>>>>>>> try_reserve_range().  I can't figure out why it should fall 
>>>>>>> through....
>>>>>>>
>>>>>>> I was expecting something like:
>>>>>>>
>>>>>>>          if (PrintCompressedOopsMode && Verbose) {
>>>>>>>            tty->print(" == H E A P B A S E M I N A D D R E S S 
>>>>>>> ==\n");
>>>>>>>          }
>>>>>>>          get the heap at aligned HeapBaseMinAddress, return if 
>>>>>>> success...
>>>>>>>
>>>>>>>          if (PrintCompressedOopsMode && Verbose) {
>>>>>>>            tty->print(" == U N S C A L E D ==\n");
>>>>>>>          }
>>>>>>>          try to get heap base for unscaled access, return if 
>>>>>>> successful
>>>>>>>
>>>>>>>          etc.
>>>>>>>
>>>>>>>
>>>>>>> You should make this into a little function for each return and 
>>>>>>> for the
>>>>>>> end of the initialize function, or move it to the ReservedHeapSpace
>>>>>>> constructor (caller).
>>>>>>>
>>>>>>>        assert(_base == NULL ||
>>>>>>> markOopDesc::encode_pointer_as_mark(_base)->decode_pointer() == 
>>>>>>> _base,
>>>>>>>               "area must be distinguishable from marks for 
>>>>>>> mark-sweep");
>>>>>>>        assert(_base == NULL ||
>>>>>>> markOopDesc::encode_pointer_as_mark(&_base[size])->decode_pointer() 
>>>>>>> ==
>>>>>>> &_base[size],
>>>>>>>               "area must be distinguishable from marks for 
>>>>>>> mark-sweep");
>>>>>>> }
>>>>>>>
>>>>>>> I ran this through JPRT and this line didn't compile on macos:
>>>>>>>
>>>>>>>        const uint64_t num_attempts_to_try   = MIN2(HeapSearchSteps,
>>>>>>> num_attempts_possible);
>>>>>>>
>>>>>>> I'm retrying now as:
>>>>>>>
>>>>>>>        const uint64_t num_attempts_to_try   =
>>>>>>> MIN2((uint64_t)HeapSearchSteps, num_attempts_possible);
>>>>>>>
>>>>>>> Sorry for the delay looking at this.  This is a big change that I
>>>>>>> needed
>>>>>>> to make more time to read.   I am pleased that it's a performance
>>>>>>> improvement.
>>>>>>>
>>>>>>> Thanks,
>>>>>>> Coleen
>>>>>>>
>>>>>>> On 12/8/14, 10:54 AM, Lindenmaier, Goetz wrote:
>>>>>>>> Hi,
>>>>>>>>
>>>>>>>> This is just a ping to gc/rt mailing lists to reach appropriate
>>>>>>>> people.
>>>>>>>>
>>>>>>>> I please need a reviewer from gc or rt, could somebody have a
>>>>>>>> look at this?
>>>>>>>>
>>>>>>>> Short summary:
>>>>>>>> - new cOops mode disjointbase that allows optimizations on PPC
>>>>>>>> improving over heapbased
>>>>>>>> - search for heaps: finds zerobased on sparc Solaris 11 and Aix
>>>>>>>> - concentrate cOops heap allocation code in one function
>>>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.02/ 
>>>>>>>>
>>>>>>>>
>>>>>>>> Please reply only to the original thread in hotspot-dev to keep 
>>>>>>>> this
>>>>>>>> local.
>>>>>>>>
>>>>>>>> Thanks and best regards,
>>>>>>>>        Goetz.
>>>>>>>>
>>>>>>>> -----Original Message-----
>>>>>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>>>>>> Sent: Donnerstag, 4. Dezember 2014 19:44
>>>>>>>> To: Lindenmaier, Goetz
>>>>>>>> Cc: 'hotspot-dev developers'
>>>>>>>> Subject: Re: RFR(L): 8064457: Introduce compressed oops mode
>>>>>>>> "disjoint base" and improve compressed heap handling.
>>>>>>>>
>>>>>>>> This looks good to me.
>>>>>>>>
>>>>>>>> Now we need second review since changes are significant. 
>>>>>>>> Preferable
>>>>>>>> from
>>>>>>>> GC group since you changed ReservedHeapSpace. They will be 
>>>>>>>> affected
>>>>>>>> most. Review from Runtime is also welcome.
>>>>>>>>
>>>>>>>> Thanks,
>>>>>>>> Vladimir
>>>>>>>>
>>>>>>>> On 12/4/14 10:27 AM, Lindenmaier, Goetz wrote:
>>>>>>>>> Hi Vladimir.
>>>>>>>>>
>>>>>>>>> Sorry.  I updated the webrev once more.  Hope it's fine now.
>>>>>>>>> At least I can write comments :)
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>>         Goetz
>>>>>>>>>
>>>>>>>>> -----Original Message-----
>>>>>>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>>>>>>> Sent: Thursday, December 04, 2014 5:54 PM
>>>>>>>>> To: Lindenmaier, Goetz
>>>>>>>>> Cc: 'hotspot-dev developers'
>>>>>>>>> Subject: Re: RFR(L): 8064457: Introduce compressed oops mode
>>>>>>>>> "disjoint base" and improve compressed heap handling.
>>>>>>>>>
>>>>>>>>> I spotted an other bug.
>>>>>>>>> You replaced !_base with _base != NULL when moved code to
>>>>>>>>> try_reserve_range() - it should be _base == NULL.
>>>>>>>>> The same problem in asserts:
>>>>>>>>>
>>>>>>>>> +  assert(_base != NULL ||
>>>>>>>>> markOopDesc::encode_pointer_as_mark(_base)->decode_pointer() ==
>>>>>>>>> _base,
>>>>>>>>> +         "area must be distinguishable from marks for 
>>>>>>>>> mark-sweep");
>>>>>>>>> +  assert(_base != NULL ||
>>>>>>>>> markOopDesc::encode_pointer_as_mark(&_base[size])->decode_pointer() 
>>>>>>>>> ==
>>>>>>>>> &_base[size],
>>>>>>>>> +         "area must be distinguishable from marks for 
>>>>>>>>> mark-sweep");
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Also you did not remove  _base && in next place:
>>>>>>>>>
>>>>>>>>> +         (_base && _base + size > zerobased_max))) {  // 
>>>>>>>>> Unscaled
>>>>>>>>> delivered an arbitrary address.
>>>>>>>>>
>>>>>>>>> New comment is good.
>>>>>>>>>
>>>>>>>>> Thanks,
>>>>>>>>> Vladimri
>>>>>>>>>
>>>>>>>>> On 12/4/14 1:45 AM, Lindenmaier, Goetz wrote:
>>>>>>>>>> Hi Vladimir,
>>>>>>>>>>
>>>>>>>>>>> Add more extending comment explaining that.
>>>>>>>>>> The comment for try_reserve_heap was meant to explain that.
>>>>>>>>>> I further added a comment in initialize_compressed_heap().
>>>>>>>>>>
>>>>>>>>>>> You need another parameter to pass UnscaledOopHeapMax or
>>>>>>>>>>> zerobased_max.
>>>>>>>>>> Oh, thanks a lot!  That's important. Fixed.
>>>>>>>>>>
>>>>>>>>>>> I mean that you already checked _base == NULL so on other side
>>>>>>>>>>> of || _base != NULL - why you need (_base &&) check?
>>>>>>>>>> Sorry, now I got it.  Removed.
>>>>>>>>>>
>>>>>>>>>> I updated the webrev:
>>>>>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.02/ 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Increment on top of the increment :)
>>>>>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.02/incremental_diffs2.patch 
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>>          Goetz.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> -----Original Message-----
>>>>>>>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>>>>>>>> Sent: Mittwoch, 3. Dezember 2014 18:32
>>>>>>>>>> To: Lindenmaier, Goetz; 'hotspot-dev at openjdk.java.net'
>>>>>>>>>> Subject: Re: RFR(L): 8064457: Introduce compressed oops mode
>>>>>>>>>> "disjoint base" and improve compressed heap handling.
>>>>>>>>>>
>>>>>>>>>> Comments are below.
>>>>>>>>>>
>>>>>>>>>> On 12/3/14 5:49 AM, Lindenmaier, Goetz wrote:
>>>>>>>>>>> Hi Vladimir,
>>>>>>>>>>>
>>>>>>>>>>> thanks for looking at the change!  See my comments inline 
>>>>>>>>>>> below.
>>>>>>>>>>>
>>>>>>>>>>> I made a new webrev:
>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.02/ 
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Incremental changes:
>>>>>>>>>>> http://cr.openjdk.java.net/~goetz/webrevs/8064457-disjoint/webrev.02/incremental_diffs.patch 
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>> Best regards,
>>>>>>>>>>>           Goetz.
>>>>>>>>>>>
>>>>>>>>>>>> -----Original Message-----
>>>>>>>>>>>> From: Vladimir Kozlov [mailto:vladimir.kozlov at oracle.com]
>>>>>>>>>>>> Sent: Mittwoch, 3. Dezember 2014 00:46
>>>>>>>>>>>> To: Lindenmaier, Goetz; 'hotspot-dev at openjdk.java.net'
>>>>>>>>>>>> Subject: Re: RFR(L): 8064457: Introduce compressed oops mode
>>>>>>>>>>>> "disjoint base" and improve compressed heap handling.
>>>>>>>>>>>>
>>>>>>>>>>>> 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=...
>>>>>>>>>>> Yes, that's good for me. Fixed.
>>>>>>>>>>>
>>>>>>>>>>>> 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?
>>>>>>>>>>> Fixed.
>>>>>>>>>>>
>>>>>>>>>>>> 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.
>>>>>>>>>>> Fixed.
>>>>>>>>>>>
>>>>>>>>>>>> In initialize_compressed_heap() again (!_base).
>>>>>>>>>>> Fixed.
>>>>>>>>>>>
>>>>>>>>>>>> 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.
>>>>>>>>>>> No, I can't and don't want to check for _base != NULL.
>>>>>>>>>>> I always keep the result of the last try, also if it didn't
>>>>>>>>>>> fulfil the required properties.
>>>>>>>>>>> So I take that result and go into the next check. That check
>>>>>>>>>>> might succeed
>>>>>>>>>>> with the heap allocated before.
>>>>>>>>>>> This allows me to separate allocation and placement criteria,
>>>>>>>>>>> and to have the
>>>>>>>>>>> placement criteria checked in only one place (per mode).
>>>>>>>>>>> Only for HeapBaseMinAddress I don't do it that way, I 
>>>>>>>>>>> explicitly
>>>>>>>>>>> call release().
>>>>>>>>>>> This way I can enforce mode heapbased.
>>>>>>>>>> I see what you are saying. It was not clear from comments 
>>>>>>>>>> what is
>>>>>>>>>> going on.
>>>>>>>>>> Add more extending comment explaining that.
>>>>>>>>>>
>>>>>>>>>>>> num_attempts calculation and while() loop are similar in
>>>>>>>>>>>> unscaled and
>>>>>>>>>>>> zerobased cases. Could you move it into a separate method?
>>>>>>>>>>> I can do that, but I don't like it as I have to pass in 7
>>>>>>>>>>> parameters.
>>>>>>>>>> You need an other parameter to pass UnscaledOopHeapMax or
>>>>>>>>>> zerobased_max.
>>>>>>>>>>
>>>>>>>>>>> That makes the code not much more readable.  The function will
>>>>>>>>>>> look like this:
>>>>>>>>>> I think initialize_compressed_heap() is more readable now.
>>>>>>>>>>
>>>>>>>>>>> void ReserveHeapSpace::try_reserve_range(char *const
>>>>>>>>>>> highest_start, char *lowest_start, size_t 
>>>>>>>>>>> attach_point_alignment,
>>>>>>>>>>> char
>>>>>>>>>>> *aligned_HBMA, size_t size, size_t alignment, bool large) {
>>>>>>>>>>>           guarantee(HeapSearchSteps > 0, "Don't set 
>>>>>>>>>>> HeapSearchSteps
>>>>>>>>>>> to 0");
>>>>>>>>>>>
>>>>>>>>>>>           const size_t attach_range = highest_start - 
>>>>>>>>>>> lowest_start;
>>>>>>>>>>>           // Cap num_attempts at possible number.
>>>>>>>>>>>           // At least one is possible even for 0 sized 
>>>>>>>>>>> attach range.
>>>>>>>>>>>           const uint64_t num_attempts_possible = 
>>>>>>>>>>> (attach_range /
>>>>>>>>>>> attach_point_alignment) + 1;
>>>>>>>>>>>           const uint64_t num_attempts_to_try =
>>>>>>>>>>> MIN2(HeapSearchSteps, num_attempts_possible);
>>>>>>>>>>>
>>>>>>>>>>>           const size_t stepsize = align_size_up(attach_range /
>>>>>>>>>>> num_attempts_to_try, attach_point_alignment);
>>>>>>>>>>>
>>>>>>>>>>>           // Try attach points from top to bottom.
>>>>>>>>>>>           char* attach_point = highest_start;
>>>>>>>>>>>           while (attach_point >= lowest_start &&
>>>>>>>>>>>                  attach_point <= highest_start &&  // Avoid 
>>>>>>>>>>> wrap
>>>>>>>>>>> around.
>>>>>>>>>>>                  (!_base || _base < aligned_HBMA || _base + 
>>>>>>>>>>> size >
>>>>>>>>>>> (char *)UnscaledOopHeapMax)) {
>>>>>>>>>>>             try_reserve_heap(size, alignment, large, 
>>>>>>>>>>> attach_point);
>>>>>>>>>>>             attach_point -= stepsize;
>>>>>>>>>>>           }
>>>>>>>>>>> }
>>>>>>>>>>>
>>>>>>>>>>>
>>>>>>>>>>>> In disjointbase while() condition no need for _base second 
>>>>>>>>>>>> check:
>>>>>>>>>>>> +           (_base == NULL ||
>>>>>>>>>>>> +            ((_base + size > (char *)OopEncodingHeapMax) &&
>>>>>>>>>>> I need this for the same reason as above: This is the check for
>>>>>>>>>>> successful allocation.
>>>>>>>>>> I mean that you already checked _base == NULL so on other 
>>>>>>>>>> side of
>>>>>>>>>> || _base != NULL - why you need (_base &&) check?
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> Vladimir
>>>>>>>>>>
>>>>>>>>>>> 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.
>>>>>>>>>>>>>
>>>>>>>>>>>>>
>



More information about the hotspot-runtime-dev mailing list