[jdk17u-dev] RFR: 8292989: Avoid dynamic memory in AsyncLogWriter

[Aleksey Shipilev]

On Fri, 28 Jul 2023 08:18:46 GMT, Xin Liu <xliu at openjdk.org> wrote:

> This patch is *NOT* a clean backport.  The logic part is exactly the same, but I have to adjust code in the following parts:
> 1. change from KVHashtable to ResourceHashtable because we want to use resourceArea instead of C_HEAP. 
> 2. we added lambda-API iterator() to ResourceHashtable. Instead of backporting all patches from TIP, we just reuse the original iterator(). 
> 
> 
> template<typename Function>
> void iterate(Function function) const { // lambda enabled API
> 
> 
> Testing: 
> jdk-test1 tests including gtest. 
> Manually check there's no malloc/free in runtime.

Looked through the code, I have comments.

src/hotspot/share/logging/logAsyncWriter.hpp line 63:

> 61:   using AsyncLogMap = ResourceHashtable<LogFileStreamOutput*, uint32_t, primitive_hash<LogFileStreamOutput*>,
> 62:                                         primitive_equals<LogFileStreamOutput*>, 17,
> 63:                                         ResourceObj::C_HEAP, mtLogging>;

These arguments should be on the new lines each, I think? This would match the upstream code better.

src/hotspot/share/utilities/resourceHash.hpp line 187:

> 185:   template<class ITER>
> 186:   void iterate(ITER* iter) const {
> 187:     auto function = [&] (K& k, V& v) {

Can we avoid doing this by making a named `struct` implementing `do_entry` for every use? I realize that would deviate the _uses_ from the upstream, but we would not be changing the shared code, which would be safer for generic code that does not use any of async logging *and* uses older toolchains.

This hunk effectively backports https://bugs.openjdk.org/browse/JDK-8276789, which seems to imply the need to check this works well with different toolchains.

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

PR Review: https://git.openjdk.org/jdk17u-dev/pull/1630#pullrequestreview-1557685667
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/1630#discussion_r1281063416
PR Review Comment: https://git.openjdk.org/jdk17u-dev/pull/1630#discussion_r1281066509