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

[David Holmes]

On 28/01/2016 3:40 AM, Daniel D. Daugherty wrote:
> 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?

Thanks Dan!

http://cr.openjdk.java.net/~dholmes/6515172/webrev.v3/

I hadn't realized I could do that! There are a few flags in globals.hpp 
that should be elsewhere.

One glitch is that the per-os flag definitions are not set up to handle 
"experimental". I did not want to have to change all of the platform 
files to deal with that, so I switched back to a diagnostic option. I 
hope that is okay? (Comment was updated in os_linux.cpp)

I also want to note that Thomas Stuefe raised an issue with the use of 
strerror() - it may not be thread-safe. We already use it quite a lot in 
the VM code so I think there needs to be a general solution so I will 
file a separate CR. I also note that it will make the logging use of 
strerror very awkward if we need to define a local buffer, and not have 
any impact when logging is disabled. :(

Thanks,
David

> 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
>>>>
>>>
>