RFR: 8248132: ZGC: Unify handling of all OopStorage instances in root processing

Per Liden per.liden at oracle.com
Tue Jun 23 14:29:46 UTC 2020


Hi,

On 6/23/20 4:09 PM, Kim Barrett wrote:
>> On Jun 23, 2020, at 9:09 AM, Per Liden <per.liden at oracle.com> wrote:
>>
>> Hi Stefan,
>>
>> On 6/23/20 10:10 AM, Stefan Karlsson wrote:
>>> Hi all,
>>> Please review this patch to unify handling of all OopStorage instances in root processing.
>>> https://cr.openjdk.java.net/~stefank/8248132/webrev.01/
>>
>> I note that the size of the dynamic allocation is always known at compile-time. So how about we just avoid it altogether with something like this?
>>
>> diff --git a/src/hotspot/share/gc/shared/oopStorageSetParState.hpp b/src/hotspot/share/gc/shared/oopStorageSetParState.hpp
>> --- a/src/hotspot/share/gc/shared/oopStorageSetParState.hpp
>> +++ b/src/hotspot/share/gc/shared/oopStorageSetParState.hpp
>> @@ -26,16 +26,16 @@
>> #define SHARE_GC_SHARED_OOPSTORAGESETPARSTATE_HPP
>>
>> #include "gc/shared/oopStorageParState.hpp"
>> +#include "gc/shared/oopStorageSet.hpp"
>>
>> template <bool concurrent, bool is_const>
>> class OopStorageSetStrongParState {
>> private:
>>    typedef OopStorage::ParState<concurrent, is_const> ParStateType;
>>
>> -  ParStateType* _par_states;
>> +  char _par_states[sizeof(ParStateType) * OopStorageSet::strong_count];
> 
> (Not a review, just a drive-by comment.)
> 
> This doesn't guarantee proper alignment of _par_states; with
> _par_states being the only member, it only requires char alignment.
> (It might happen to work because of vagaries of heap allocators and
> stack alignment requirements, possibly always on some platforms, but
> that's not guaranteed.)
> 
> -  ParStateType* _par_states;
> +  char _par_states[sizeof(ParStateType) * OopStorageSet::strong_count];

You're right, but I'm thinking ATTRIBUTE_ALIGNED should work here.

cheers,
Per


> 
> Doing that correctly is a bit annoying, which is why C++11 added
> std::aligned_storage.
> 


More information about the hotspot-gc-dev mailing list