[8u] RFR for JDK-8154324: Request to backport JDK-6515172 to 8u

[David Holmes]

Hi Shafi,

On 12/09/2016 11:20 PM, Shafi Ahmad wrote:
> Hi,
>
> Please find updated webrev: http://cr.openjdk.java.net/~shshahma/8154324/webrev.01/
> I have incorporated both review comment by David.
>   1. Removed unrelated comment.
>   2. Removed unreferenced code ' diagnostic(bool, UseCpuAllocPath, false,'

Incorrect comment here in globals_linux.hpp:

!   diagnostic(bool, PrintActiveCpus, false,                              \
!              "Use CPU_ALLOC code path in os::active_processor_count ")

should be:

"Print the number of CPUs detected in os::active_processor_count"

---

os_linux.cpp:

if(PrintActiveCpus) {

Need space after if.

--

Otherwise okay. No need to see updated webrev if above are fixed.

Thanks,
David

>
> Regards,
> Shafi
>
>> -----Original Message-----
>> From: David Holmes
>> Sent: Friday, September 09, 2016 11:29 AM
>> To: Shafi Ahmad; hotspot-dev at openjdk.java.net
>> Subject: Re: [8u] RFR for JDK-8154324: Request to backport JDK-6515172 to
>> 8u
>>
>> Hi Shafi,
>>
>> On 9/09/2016 3:46 PM, Shafi Ahmad wrote:
>>> Hi,
>>>
>>> Please review the backport [modified] of bug: "JDK-6515172
>> Runtime.availableProcessors() ignores Linux taskset command" to jdk8u.
>>>
>>> Please note that the backport is not clean and we can't do as it is. Please
>> note
>>>   1. The changes are made by 'Andreas Eriksson' who left Oracle.
>>>   2. There is difference in logging mechanism in jdk9 and jdk8 is different
>> and file logTag.hpp doesn't exists in jdk8.
>>>   3. Newly added test pass after this change. It fails without this change.
>>>
>>> Webrev link: http://cr.openjdk.java.net/~shshahma/8154324/webrev.00/
>>> Jdk9 bug: https://bugs.openjdk.java.net/browse/JDK-6515172
>>> Jdk8 bug: https://bugs.openjdk.java.net/browse/JDK-8154324
>>> Original patch pushed to jdk9:
>>> http://hg.openjdk.java.net/jdk9/jdk9/hotspot/rev/c5480d4abfe4
>>
>> I worked extensively with Andreas on this as there were a number of issues.
>> I'll have to try and find those discussions to see where we ended up.
>>
>> The backport as stands is not quite appropriate. For example it adds:
>>
>> diagnostic(bool, UseCpuAllocPath, false,
>>
>> but that does not exist in the actual code for 8.
>>
>> Also this comment:
>>
>> + // If it appears there may be more than 1024 processors then we do a
>> + // dynamic check - see 6515172 for details.
>>
>> is wrong as there is no dynamic check in this version of the code.
>>
>> The last I recall with this is that there were issues caused by building with one
>> version of glibc and running (or trying to) on later versions of glibc. But as I
>> said I will have to see if I have the discussion from that effort.
>>
>> David
>> ----
>>
>>> Testing: jprt and running jtreg.
>>>
>>> Regards,
>>> Shafi
>>>