RFR: 8205683: Refactor heap allocation to separate concerns

Erik Österlund erik.osterlund at oracle.com
Wed Jun 27 14:58:53 UTC 2018


Hi Per,

Thank you for the review.

/Erik

On 2018-06-27 16:26, Per Liden wrote:
> Hi Erik,
>
> On 2018-06-27 15:23, Erik Österlund wrote:
>> Hi Per,
>>
>> Thank you for reviewing.
>>
>> Incremental:
>> http://cr.openjdk.java.net/~eosterlund/8205683/webrev.02_03/
>>
>> Full:
>> http://cr.openjdk.java.net/~eosterlund/8205683/webrev.03/
>> [...]
>>> I doesn't look like the above functions need to take in TRAPS, right?
>>
>> It is used by callers so they can use the CHECK_NULL macro on the 
>> callsite. But inside, I don't need it.
>
> Ah, I see. I tend to think that we're only using that style when 
> something inside throws and exception, but anyway, keep it as is.
>
> [...]
>>> src/hotspot/share/gc/shared/threadLocalAllocBuffer.hpp
>>> ------------------------------------------------------
>>>
>>> 143   HeapWord* allocate_sampled_object(size_t size);
>>>
>>> Can we call this function allocate_sampled() to better match with 
>>> allocate()?
>>
>> Not sure where you found this function; I have removed it. The logic was 
>
> You're right of course, ignore me.
>
>> instead moved into the allocate_inside_tlab_slow function. One 
>> alternative I considered is to pass in a bool* to 
>> ThreadLocalAllocBuffer::allocate() that if not NULL also tries to 
>> bump the end to its actual end, and returns back whether the end was 
>> bumped by the sampling logic or not inside of allocate, and have that 
>> pointer point straight into Allocation::_tlab_end_reset_for_sample. 
>> If you prefer that style, I can change it to do that instead.
>
> I like what you have now, so I'd say keep it as is.
>
> In summary, looks good, ship it!
>
> cheers,
> Per
>
>>
>> Thanks,
>> /Erik
>>
>>>
>>> cheers,
>>> Per
>>




More information about the hotspot-gc-dev mailing list