RFR: 6515172: Runtime.availableProcessors() ignores Linux taskset command

[Daniel D. Daugherty]

On 1/27/16 12:16 AM, David Holmes wrote:
> Okay here is v2:
>
> http://cr.openjdk.java.net/~dholmes/6515172/webrev.v2/

Reviewed by comparing the two patch files. Looks good modulo
one question: should the change to src/share/vm/runtime/globals.hpp
be in src/os/linux/vm/globals_linux.hpp instead?

Dan


>
> - UseNewCode is replaced by a new experimental flag UseCpuAllocPath
> - the logging statement shows whether the path was forced to be taken
> - this is now enabled in product builds
>
> Sorry I don't know how to generate an incremental webrev when working 
> with uncommitted changes. :(
>
> Thanks,
> David
>
> On 27/01/2016 9:20 AM, Gerald Thornbrugh wrote:
>> Hi David,
>>
>> I also like the idea of using an "-XX" option, that seems like the best
>> way to go.
>>
>> I am ok with leaving the code in.  If there are any issues with the code
>> we can always
>> create a bug and remove it.
>>
>> Jerry
>>> On 1/26/16 2:12 PM, David Holmes wrote:
>>>> Update ...
>>>>
>>>> On 22/01/2016 6:06 PM, David Holmes wrote:
>>>>> First a special thanks to Martin Buchholz for his input, feedback,
>>>>> critique and raising awareness of how non-simple this issue is.
>>>>>
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-6515172
>>>>> webrev: http://cr.openjdk.java.net/~dholmes/6515172/webrev/
>>>>>
>>>>> Basic problem:
>>>>>    processors available for use <= processors online <= processors
>>>>> configured
>>>>>
>>>>> but we always returned the number of online processors.
>>>>>
>>>>> Solution is simple in its basic form: use sched_getaffinity to get 
>>>>> the
>>>>> scheduling affinity mask and count the number of available 
>>>>> processors.
>>>>>
>>>>> Details are complicated by the desire to handle very large processor
>>>>> systems. See the bug report for lots of detailed discussions and
>>>>> references.
>>>>>
>>>>> Testing:
>>>>>   - new test that verifies behaviour when running under taskset
>>>>>   - diagnostic hook injection (UseNewCodeN) to enable testing of all
>>>>> code paths (one hook is left in for non-product to allow easy
>>>>> testing of
>>>>> the dynamic path)
>>>>
>>>> I have been told that using the development flag UseNewCode in
>>>> released code is a bad idea because it is used internally during
>>>> development (as per its defined purpose).
>>>>
>>>> I would like to be able to test the dynamic path easily, but I didn't
>>>> want to pay the price of adding a new VM option to do it. So choices
>>>> are:
>>>>
>>>> a) don't do anything (remove the UseNewCode check)
>>>> b) add a new diagnostic flag
>>>> c) add a new experimental flag
>>>
>>> My vote:
>>>
>>> -XX:+UnlockExperimentalVMOptions -XX:+DynamicConfiguredCPUPath
>>>
>>> With a plan (and a bug) to remove that option down the road.
>>>
>>> And a code review.
>>>
>>> src/os/linux/vm/os_linux.cpp
>>>     No comments.
>>>
>>> src/share/vm/logging/logTag.hpp
>>>     No comments.
>>>
>>> test/runtime/os/AvailableProcessors.java
>>>     Nice test. I like that the default (no args) runs a set of tests
>>>     and if you (manually) run the test with a specific arg it will
>>>     check just that one value.
>>>
>>> Thumbs up! (Assuming you change UseNewCode to something else or
>>> delete its use all together.
>>>
>>> Dan
>>>
>>>
>>>>
>>>> Thoughts?
>>>>
>>>> Thanks,
>>>> David
>>>>
>>>>
>>>>>   - JPRT
>>>>>
>>>>> Compatability issues:
>>>>> - the system code we're using now is at least 5 years old so distro's
>>>>> older than that (which are not officially supported) may not work
>>>>> - anyone already running under a processor constrained environment
>>>>> (like
>>>>> Docker) and using availableProcessor() to "size" things, will find 
>>>>> that
>>>>> size has now changed. We do not expect this to be a problem - on the
>>>>> contrary we expect Docker users to want the new behaviour.
>>>>>
>>>>> Thanks,
>>>>> David
>>>
>>