On Wed, 11 Jan 2023 15:08:23 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:

> > ArrayAllocator with ArrayAllocatorMallocLimit removed is effectively 
> > MallocArrayAllocator. Are you suggesting leaving MallocArrayAllocator and 
> > MmapArrayAllocator thus update references to ArrayAllocator to be 
> > MallocArrayAllocator?
> 
> Yes, that was what I wanted.
> 
> > As far as APIs, the majority of the codebase uses the macros. IMO 
> > consistency would be better and having two ways of doing things doesn't 
> > help. But if you feel strongly about it, we can punt and just surgically 
> > remove the bare minimum, assuming you clarify your expectation (see above).
> 
> I agree about the argument about consistency, so I retract my objection. We 
> can deal with these macros as a separate RFE (if we ever get to it).
> 
> I would like to retain the usage of mmapped memory for ZGC to minimize the 
> risk of any surprises for us. ZGC code also tend to initialize as much as 
> possible in the initialization list, so the extra memset that comes after 
> initialization lists sticks out a bit. Could you create a private 
> `ZGranuleMap::allocate_array` function that uses 
> os::reserve_memory/commmit_memory and change the code to be:
> 
> ```
> inline ZGranuleMap<T>::ZGranuleMap(size_t max_offset) :
>     _size(max_offset >> ZGranuleSizeShift),
>     _map(allocate_array(_size)) {
> ```
> 
> Or if you like, I can provide a patch on top of your branch to do that.

The extra memset is due to `malloc` not handing out zero initialized memory, it 
looks like `zGranuleMap` is sensitive to that.

Done. Restored mmap as suggested for zGranuleMap.

-------------

PR: https://git.openjdk.org/jdk/pull/11931

Reply via email to