RFR: 8299915: Remove ArrayAllocatorMallocLimit and associated code [v10]

[Thomas Stuefe]

On Thu, 19 Jan 2023 17:23:19 GMT, Justin King <jcking at openjdk.org> wrote:

>> Remove abstraction that is a holdover from Solaris. Direct usages of `MmapArrayAllocator` have been switched to normal `malloc`. The justification is that none of the code paths are called from signal handlers, so using `mmap` directly does not make sense and is potentially slower than going through `malloc` which can potentially re-use memory without making any system calls. The remaining usages of `ArrayAllocator` and `MallocArrayAllocator` are equivalent.
>
> Justin King has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Do not pass nullptr to os::release_memory
>   
>   Signed-off-by: Justin King <jcking at google.com>

Cursory review

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 125:

> 123:   return true;
> 124: }
> 125: 

Here, and in ZGC: we are cool with small allocations being rounded up to page size now?

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 131:

> 129:   void* const addr = os::reserve_memory(size, !ExecMem, mtGC);
> 130:   if (addr == nullptr) {
> 131:     return nullptr;

reserve fails, we return null, commit fails, we exit? Why the inconsistency?

src/hotspot/share/gc/g1/g1ConcurrentMark.cpp line 150:

> 148: 
> 149: size_t G1CMMarkStack::size_for_array(size_t count) {
> 150:   return align_up(count * sizeof(TaskQueueEntryChunk), os::vm_allocation_granularity());

assert against overflow

src/hotspot/share/gc/z/zGranuleMap.inline.hpp line 79:

> 77: template <typename T>
> 78: size_t ZGranuleMap<T>::size_for_array(size_t count) {
> 79:   return align_up(count * sizeof(T), os::vm_allocation_granularity());

assert ag. overflow?

src/hotspot/share/memory/padded.inline.hpp line 70:

> 68:   // Clear the allocated memory.
> 69:   memset(chunk, '\0', total_size);
> 70: 

Old code, when doing the malloc path, did not initialize. Why here?

test/lib-test/jdk/test/whitebox/vm_flags/SizeTTest.java line 38:

> 36: 
> 37: public class SizeTTest {
> 38:     private static final String FLAG_NAME = "StringDeduplicationCleanupDeadMinimum";

Small comment here that the flag itself, does not matter, just the fact that it is of type size_t?

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

PR Review: https://git.openjdk.org/jdk/pull/11931#pullrequestreview-1437169771
PR Review Comment: https://git.openjdk.org/jdk/pull/11931#discussion_r1200844220
PR Review Comment: https://git.openjdk.org/jdk/pull/11931#discussion_r1200842961
PR Review Comment: https://git.openjdk.org/jdk/pull/11931#discussion_r1200845162
PR Review Comment: https://git.openjdk.org/jdk/pull/11931#discussion_r1200840143
PR Review Comment: https://git.openjdk.org/jdk/pull/11931#discussion_r1200851869
PR Review Comment: https://git.openjdk.org/jdk/pull/11931#discussion_r1200852952