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

Kim Barrett kim.barrett at oracle.com
Tue Jun 23 14:09:41 UTC 2020


> 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];

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