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