[jdk17u-dev] RFR: 8305995: Footprint regression from JDK-8224957

[Kirill A. Korinsky]

On Fri, 5 May 2023 06:12:17 GMT, Goetz Lindenmaier <goetz at openjdk.org> wrote:

>> @tstuefe force push triggers checks
>
> Hi @catap, you may not integrate before you have jdk17u-fix-yes on the JBS issue.
> 
> Please 
>  * tell what testing you did in addition to GHA. GHA is only a very basic sanity test.
>  * Reason why you want to have it in 17.0.8.  We usually push to the same release as Oracle.
> (both this should go into the JBS issue, but I will look here as you don't have access to JBS)
> I'm removing the tag in the meantime. Add it again after supplying the information.

Hi @GoeLin,

This regression was found while trying to migrate an application from JDK 1.8 to JDK 17, by running internal benchmarks and investigating abnormal memory usage for about 4 times more of one of them. This application is quite memory sensitive.

Outside of this application, I've discovered a lot of small wired things that are affected by this regression.

For example, I can't compile from IntelliJ IDEA [fastutils](https://github.com/vigna/fastutil) when using JDK17 or JDK21, but I can when using JDK11. After my fix I can use both JDK17 and JDK21.

As part of the investigation I've been able to implement a benchmark that proves that the regression exists and allows to track that it has been fixed.

The idea of the benchmark is to build an RB tree containing a lot of primitive `int` and use JMH's GC profiler to measure the footprint.

When I run `make test TEST="micro:RBTreeSearch" MICRO_OPTIONS="-prof gc"` without backport of my fix I get an output:

Benchmark                                              Mode  Cnt     Score     Error   Units
RBTreeSearch.search                                   thrpt   12     0.068 ?   0.002  ops/us
RBTreeSearch.search:?gc.alloc.rate                    thrpt   12   259.208 ?   7.126  MB/sec
RBTreeSearch.search:?gc.alloc.rate.norm               thrpt   12  5024.473 ?   0.151    B/op
RBTreeSearch.search:?gc.churn.G1_Eden_Space           thrpt   12   249.292 ?  44.106  MB/sec
RBTreeSearch.search:?gc.churn.G1_Eden_Space.norm      thrpt   12  4834.329 ? 865.041    B/op
RBTreeSearch.search:?gc.churn.G1_Survivor_Space       thrpt   12     0.007 ?   0.015  MB/sec
RBTreeSearch.search:?gc.churn.G1_Survivor_Space.norm  thrpt   12     0.139 ?   0.284    B/op
RBTreeSearch.search:?gc.count                         thrpt   12    25.000            counts
RBTreeSearch.search:?gc.time                          thrpt   12    20.000                ms

and to compare with this fix:

Benchmark                                 Mode  Cnt  Score    Error   Units
RBTreeSearch.search                      thrpt   12  0.102 ?  0.003  ops/us
RBTreeSearch.search:?gc.alloc.rate       thrpt   12  0.002 ?  0.001  MB/sec
RBTreeSearch.search:?gc.alloc.rate.norm  thrpt   12  0.027 ?  0.001    B/op
RBTreeSearch.search:?gc.count            thrpt   12    ? 0           counts


As you can see, the footprint is quite different. It is almost zero, as expected for such a data structure using only primitive `int`, but after the fix. As Vladimir Kozlov pointed out, let me quote:

> It shows that C2 did not eliminate Integer boxing allocations without your fix. As result they consume Java heap. With your fix C2's Escape Analysis eliminated these allocations.

>From my point of view, this regression is a major issue that can improve the performance of a lot of applications, especially those that are sensitive to latency in general or the footprint in particular.

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

PR Comment: https://git.openjdk.org/jdk17u-dev/pull/1262#issuecomment-1536101695