On Tue, 21 Jun 2022 13:44:23 GMT, Thomas Stuefe <stu...@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
