RFR: 8287135: Calculation of jmm_GetMemoryUsage is wrong [v4]

Yi Yang yyang at openjdk.org
Mon Jun 27 08:06:56 UTC 2022


On Tue, 21 Jun 2022 13:44:23 GMT, Thomas Stuefe <stuefe at openjdk.org> wrote:

>>> > > I would have thought that since we don't have the pool anymore, we can just remove this test line. The lines above already
>>> > > test against MaxMetaspaceSize.
>>> > 
>>> > 
>>> > Okay.
>>> > > I think you may be right, we need a replacement for the old memory bean for these tests. Whitebox seems easiest.
>>> > 
>>> > 
>>> > So should we keep test changes as it is or discard existing test changes and then rewrite related tests via new compressed class space query whitebox API? I prefer to keep tests as it is rather than adding whitebox API since I've made a lot of test changes. But I also want to hear your expert suggestions as final conclusion.
>>> 
>>> I think the easier way would be actually to add a whitebox API for class space use, as @iklam suggested, and just replace the memory pool usage calls with that one. That would be a purely mechanical change if a bit onerous. But since the metaspace itself did not change, the numbers are the same, so the tests test the same. Still easier than trying to think through the changed semantics for each test.
>>> 
>>> Sorry that this seems to have exploded in complexity :-(
>> 
>> Never mind:) I did a closer look at these test changes, it seems that many changes are still necessary even if we provide a WhiteBox.getCompressedClassSpaceMemoryUsage(). In particular, tests other than test/hotspot/jtreg/vmTestbase/vm/mlvm/indy/stress/gc/lotsOfCallSites/Test.java need to be tweaked. Can you please confirm it? Thanks.
>
>> > > > I would have thought that since we don't have the pool anymore, we can just remove this test line. The lines above already
>> > > > test against MaxMetaspaceSize.
>> > > 
>> > > 
>> > > Okay.
>> > > > I think you may be right, we need a replacement for the old memory bean for these tests. Whitebox seems easiest.
>> > > 
>> > > 
>> > > So should we keep test changes as it is or discard existing test changes and then rewrite related tests via new compressed class space query whitebox API? I prefer to keep tests as it is rather than adding whitebox API since I've made a lot of test changes. But I also want to hear your expert suggestions as final conclusion.
>> > 
>> > 
>> > I think the easier way would be actually to add a whitebox API for class space use, as @iklam suggested, and just replace the memory pool usage calls with that one. That would be a purely mechanical change if a bit onerous. But since the metaspace itself did not change, the numbers are the same, so the tests test the same. Still easier than trying to think through the changed semantics for each test.
>> > Sorry that this seems to have exploded in complexity :-(
>> 
>> Never mind:) I did a closer look at these test changes, it seems that many changes are still necessary even if we provide a WhiteBox.getCompressedClassSpaceMemoryUsage(). In particular, tests other than test/hotspot/jtreg/vmTestbase/vm/mlvm/indy/stress/gc/lotsOfCallSites/Test.java need to be tweaked. Can you please confirm it? 
> 
> Why, what changes do you have in mind?

Hi @tstuefe @iklam, I've added WhiteBox.getCompressedClassSpace, but it seems that new commit is not much better/more straightforward than current version.

Things are getting more complicated after adding whitebox API. We fake a compressed class pool via WhiteBox.getCompressedClassMemoryPool, but ManagementFactory.getMemoryPoolMXBeans() can not aware of this newly created pool, because its internal structures are created at VM startup. The consequence is , any test involving iterating all pools, should be tweaked as before. We did much more than before, but we benefit less from it.

Please take a look at [new version](https://urldefense.com/v3/__https://github.com/openjdk/jdk/compare/master...kelthuzadx:jmm_calc_v2?expand=1__;!!ACWV5N9M2RV99hQ!LawnKAskIviQw1F1WYg4Jqozx8LlY-cUvmvLZ61Xw0ScQVDWe6B7UU1f3KJ_djJMrZF9bRiLlFb4wo2FC-lnLdu0Lw$ ), I don't push it directly so that we have a chance to use current version.

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

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


More information about the serviceability-dev mailing list