RFR(L): 8064457: Introduce compressed oops mode "disjoint base" and improve compressed heap handling.
Coleen Phillimore
coleen.phillimore at oracle.com
Wed Dec 24 23:13:39 UTC 2014
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