[jdk8u-dev] RFR: 6899049: G1: Clean up code in ptrQueue.[ch]pp and ptrQueue.inline.hpp [v2]

[Sun Jianye]

On Wed, 20 Sep 2023 18:48:41 GMT, Andrew John Hughes <andrew at openjdk.org> wrote:

>>> Where does this test come from? I don't see it in the patch being backported.
>> 
>> We added it to check whether the problem is sovled.  I saw there are errors like `Insufficient Memory Error` or `could not reserve enough space for 2097152KB object heap`. Maybe the resources of the CI environment are limited， the size of memory  is less than 4G when the jvm starts with `-Xmx4096m`. When i change it to `-Xmx2048m`, it seems that the jvm cannot start or  run with OOM.
>> 
>> Do you think it is necessary to add this test case ? If not, i will delete it. Or  set the test  not to execute in these three scenarios(Linux x86/windows x86/Windows x64) ?
>
>> > Where does this test come from? I don't see it in the patch being backported.
>> 
>> We added it to check whether the problem is sovled. I saw there are errors like `Insufficient Memory Error` or `could not reserve enough space for 2097152KB object heap`. Maybe the resources of the CI environment are limited， the size of memory is less than 4G when the jvm starts with `-Xmx4096m`. When i change it to `-Xmx2048m`, it seems that the jvm cannot start or run with OOM.
>> 
>> Do you think it is necessary to add this test case ? If not, i will delete it. Or set the test not to execute in these three scenarios(Linux x86/windows x86/Windows x64) ?
> 
> Even with a working test, this is not the place to include something new unless it is specific to 8u.  If you do want to include it, it needs to be separated from your backport and proposed to https://github.com/openjdk/jdk under its own bug ID.  Once included there, it can be backported to 8u, as you have with JDK-6899049 here. Not only do we not want tests being unique to the 8u repository, but changes to the main repository get attention from those who are experts in this field. Stable update trees are generally expected to get fixes that have already been reviewed, but might need some minor modification to work on an older version, and so don't get the same reviewer coverage.
> 
> As to the test case itself, it's not clear to me what it's trying to test. Is it the command-line options? Or the actual allocation? I see other cases in the HotSpot tests where a 2GB heap is used, but they only run the VM with `-version` (e.g. https://github.com/openjdk/jdk/blob/HEAD/test/hotspot/jtreg/runtime/CompressedOops/UseCompressedOops.java ) 
> 
> The failure isn't architecture-specific, so the appropriate exclusion would be to check the available memory before running the VM process. If you only want to check the arguments work, then running with `-version` should be enough.

Hi Andrew,  would you please review the code changes again?  Also, are there any other committers who can help review this MR? Please help me, thx~ @gnu-andrew

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

PR Comment: https://git.openjdk.org/jdk8u-dev/pull/374#issuecomment-1758765061