review request for 6798511/6860431: Include functionality of Surrogate in Character

Ulf Zibis Ulf.Zibis at gmx.de
Fri Mar 19 20:29:37 UTC 2010


Am 17.03.2010 16:46, schrieb Martin Buchholz:
> On Wed, Mar 17, 2010 at 01:11, Ulf Zibis<Ulf.Zibis at gmx.de>  wrote:
>    
>> Am I mad ???
>>
>> 2nd. correction:
>>
>> But
>>         for (int i = offset; i<  offset + count; i++) {
>>             int c = codePoints[i];
>>             char plane = (char)(c>>>  16);
>>             if (plane == 0)
>>                 n += 1;
>>             else if (plane<  0x11)
>>                 n += 2;
>>             else throw new IllegalArgumentException(Integer.toString(c));
>>         }
>> has too only 2 branches and additionally could benefit from tiny 16-bit
>> comparisons.
>> The shift additionally could be omitted on CPU's which can benefit from
>> 6933327.
>>      
> I'm not a x86 or hotspot expert, but I would think that the "plane"
> variable is never written to memory, but lives only in a register,
> so I see only drawbacks to making plane a "char".
>    

The char is not important here, maybe give hotspot a hint that value is 
always positive 16-bit. My idea was to indicate this to the reader.

I saw, that you use to set a space after casts, why? Cast is a 
one-operand operator like - -- ++. This a rare style in the JDK sources 
which "disturbs" my eyes. ;-)

-Ulf





More information about the core-libs-dev mailing list