RFR (S): 8233702: Introduce helper function to clamp value to range

[Thomas Schatzl]

AllocatePrefetchDistance
Hi,

On 14.11.19 04:27, Kim Barrett wrote:
>> On Nov 13, 2019, at 10:23 AM, Thomas Schatzl <thomas.schatzl at oracle.com> wrote:
>>
>>   I re-added the assert, and re-checked in our CI with hs-tier1-5. For some reason there were some failures I thought I had fixed already. Sorry :(
>>
>> Here are new webrevs:
>>
>> http://cr.openjdk.java.net/~tschatzl/8233702/webrev.0_to_1/ (diff)
>> http://cr.openjdk.java.net/~tschatzl/8233702/webrev.1/ (full)
>>
>> Thanks,
>>   Thomas
> 
> ------------------------------------------------------------------------------
> src/hotspot/share/compiler/compilerDefinitions.cpp
>   355         FLAG_SET_DEFAULT(MetaspaceSize, clamp(MetaspaceSize, 12*M, MaxMetaspaceSize));
> 
> I've not found anything that guarantees MaxMetaspaceSize >= 12*M.
> 

Reverted.

> ------------------------------------------------------------------------------
> src/hotspot/share/gc/shared/threadLocalAllocBuffer.cpp
>   254   // We can't use clamp() here because min_size() and max_size() because some
> 
> s/here because min_size()/here between min_size()/

Fixed.

> 
> ------------------------------------------------------------------------------
> src/hotspot/share/runtime/globals.hpp
> 1408   product(intx,  AllocatePrefetchDistance, -1,                              \
> 1409           "Distance to prefetch ahead of allocation pointer. "              \
> 1410           "-1: use system-specific value (automatically determined")        \
> 1411           range(-1, 512)                                                    \
> 1412           constraint(AllocatePrefetchDistanceConstraintFunc,AfterMemoryInit)\
> 
> With the addition of the range restriction, is the constraint function
> still needed?  I don't remember whether a range restriction is applied
> to assignments such as are being done in various vm_version_<cpu>.cpp
> files.
> 

Yes, because the function is restricting between 0 and 512, but a -1 
input value is allowed.

But I reverted this change because ultimately it is the same issue as 
the one for MinTLABSize in 
ThreadLocalAllocBuffer::initial_desired_size() which required me to not 
use clamp() there. I.e. at that time some variables (including 
AllocatePrefetchDistance) are not stable yet.

However when I did the changes I first fixed the 
AllocatePrefetchDistance issue, then removed the clamp() there without 
reconsidering the earlier change.

New webrevs:

  http://cr.openjdk.java.net/~tschatzl/8233702/webrev.1_to_2/ (diff)
  http://cr.openjdk.java.net/~tschatzl/8233702/webrev.2/ (full)

Passed hs-tier1-5.

Thanks,
   Thomas