RFR: JDK-8281274: deal with ActiveProcessorCount in os::Linux::print_container_info [v3]

Severin Gehwolf sgehwolf at openjdk.java.net
Tue Feb 8 17:35:11 UTC 2022


On Mon, 7 Feb 2022 16:15:41 GMT, Severin Gehwolf <sgehwolf at openjdk.org> wrote:

>> Matthias Baesken has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Adjust output again, adjust test
>
> test/hotspot/jtreg/containers/docker/TestMisc.java line 89:
> 
>> 87:         Common.logNewTestCase("Test print_container_info()");
>> 88: 
>> 89:         DockerRunOptions opts = Common.newOpts(imageName, "PrintContainerInfo", "-XX:ActiveProcessorCount=2");
> 
> It would be better to separate this specific test. More like this:
> https://github.com/jerboaa/jdk/commit/3911ef81c8f280a2c0da259211b91f4c3e8c3994

I'm not sure I like this version of the test better. I'd rather separate concerns. The changes to `Common` aren't really needed, as `Common.newOpts(imageName, "PrintContainerInfo").addJavaOpts("-XX:ActiveProcessorCount=2");` can be used instead. Similarly, the changes to `checkContainerInfo()` make the test harder to read. It's once called with a `null` argument and checks similar things, but one additional assert. Rather than that, I'd just do the assert directly in the new test as suggested. This saves the reader having to look up call-sites to know what the test is actually asserting. This also more closely matches what's likely to happen in real scenarios (override the container detection heuristics with `-XX:ActiveProcessorCount`).

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

PR: https://git.openjdk.java.net/jdk/pull/7354


More information about the hotspot-runtime-dev mailing list