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

[Xueming Shen]

Martin Buchholz wrote:
> 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".
>
>   
I doubt there is any benefit to use a 8-bit or 16-bit operand on a 
32-bit/64-bit machine.
While optimization is definitely good, but it might not be a good habit 
to code in high-level
program language while thinking in assembly every each minute:-)  let's 
leave those
optimization to hotspot engineer:-)
In this particular case, given most application will never use 
supplementary character, I
doubt it really worth the optimization and I would definitely not try to 
change the impl
of isSupplementaryCP to make this code "better". If you really really 
want to optimize
this code the alternative is to have a package private 
Character.getPlane(), or simply
to use your optimized code above. I would suggest to use int for plane, 
instead of char or
byte.

-Sherman