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

[Ulf Zibis]

Am 17.03.2010 18:05, schrieb 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:-)

Yes, you are right. Unfortunately they are on delay with such things. As 
I said, the "(c >>> 16) == 0"-trick will loose it's justification, if 
JIT would be so smart, to convert a range check into a single unsigned 
compare.

-Ulf


> 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
>
>
>
>
>