RFR : 8221696: MappedByteBuffer.force method to specify range

[Mikael Vidstedt]

Would it be worth putting the logic in an aptly named (private) method? Something like:

...
private long alignDown(long address, long alignment) {
   return address & <whatever>;
}
...
long mapBase = alignDown(address, ps);
…


Cheers,
Mikael

> On May 13, 2019, at 9:14 AM, Andrew Dinn <adinn at redhat.com> wrote:
> 
> Thank you for looking at the patch.
> 
> On 28/04/2019 18:09, Andrew Haley wrote:
>> On 4/25/19 5:34 PM, Andrew Dinn wrote:
>>>   long map_base = (address & ~(ps - 1));
>> This looks like a hard way to write
>>   long map_base = address & -ps;
> 
> My version certainly uses more characters, that is for sure. However, I chose (and still prefer) this form for /clarity/. It seems to me to communicate as simply as possible what mask is being constructed, granted the starting premise that ps is a power of 2 (i.e. has only a single bit set).
> 
> It only requires elementary knowledge of binary representations to see:
> 
>  firstly, that ps-1 clears the original bit and sets all lower bits;
> 
>  next that ~(ps-1) clears those lower bits and sets all bits from the original (inclusive) upwards;
> 
>  then that the result can be used as a mask to clear those same bottom bits from address;
> 
>  finally that this mask operation is equivalent to rounding down to the relevant power of two.
> 
> I find your alternative (a tad) less clear because it employs an arithmetic operation to achieve the requisite bitwise transformation. That the two expressions compute to equivalent results requires some experience and understanding of the twos-complement representations of numbers rather than basic knowledge of bit fields.
> 
> As a gcc hacker 'your mileage may vary' ;-)
> 
> Crucially, every compiler we rely on is going to produce the same code in both cases.
> 
> regards,
> 
> 
> Andrew Dinn
> -----------
> Senior Principal Software Engineer
> Red Hat UK Ltd
> Registered in England and Wales under Company Registration No. 03798903
> Directors: Michael Cunningham, Michael ("Mike") O'Neill, Eric Shander