RFR (S): 8075288: Remove dictionary NULL check on common path of BlockFreeList methods

[Stefan Karlsson]

On 2015-04-30 16:31, Jungwoo Ha wrote:
> That was on another thread.
> Now I get that, so I'll just cc next time.

Great! Thanks.

StefanK

>
> On Thu, Apr 30, 2015 at 7:21 AM, Stefan Karlsson 
> <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>> wrote:
>
>     On 2015-04-30 16:05, Jungwoo Ha wrote:
>>     Well, I just followed what Kim suggested before.
>>     I did that before for JDK-8075288, and no one responded yet on
>>     runtime thread.
>>     There are too many different voices from Oracle, which confuses me.
>
>     Kim suggested that you should involve the Runtime team:
>
>     "I'm not sure of this, but I think metaspace belongs to runtime rather
>     than gc. If so, this should go through the hs-rt repository and have
>     runtime folks involved in the review."
>
>     not that you should create a new, separate thread on the
>     hotspot-rt list.
>
>     StefanK
>
>>
>>     On Thu, Apr 30, 2015 at 6:42 AM, Stefan Karlsson
>>     <stefan.karlsson at oracle.com <mailto:stefan.karlsson at oracle.com>>
>>     wrote:
>>
>>         Hi Jungwoo,
>>
>>
>>         On 2015-04-30 15:33, Jungwoo Ha wrote:
>>>
>>>
>>>             I agree. This is a nice cleanup, irrespective of any
>>>             potential performance gains.
>>>
>>>             http://cr.openjdk.java.net/~jwha/8079091/webrev.00/src/share/vm/memory/metaspace.cpp.udiff.html
>>>             <http://cr.openjdk.java.net/%7Ejwha/8079091/webrev.00/src/share/vm/memory/metaspace.cpp.udiff.html>
>>>
>>>             -BlockFreelist::BlockFreelist() : _dictionary(NULL) {}
>>>             +BlockFreelist::BlockFreelist()
>>>             +    : _dictionary(new BlockTreeDictionary()) {
>>>             +  assert(_dictionary != NULL, "Failed to allocate
>>>             BlockTreeDictionary");
>>>             +}
>>>
>>>             No need to NULL check CHeapObj allocations, since the
>>>             JVM will exit if it failed to get memory. See AllocateHeap:
>>>               if (p == NULL && alloc_failmode ==
>>>             AllocFailStrategy::EXIT_OOM) {
>>>             vm_exit_out_of_memory(size, OOM_MALLOC_ERROR,
>>>             "AllocateHeap");
>>>               }
>>>
>>>
>>>         http://cr.openjdk.java.net/~jwha/8079091/webrev.02/
>>>         <http://cr.openjdk.java.net/%7Ejwha/8079091/webrev.02/>
>>>
>>>         That part is taken care of on webrev.02.
>>>         The conversation is also happening at runtime mailing list
>>>         as Kim suggested to pass it to runtime.
>>
>>         Please don't split up a review request that way in the
>>         future. It would have been enough to CC the
>>         hotspot-runtime-dev list.
>>
>>         I'll leave the rest of my comments on that list.
>>
>>         Thanks,
>>         StefanK
>>
>>>         I think the general agreement is on using webrev.02.
>>>
>>
>>
>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20150430/57aef6c2/attachment.htm>