RFR (S) 8136500: Integer/Long getChars and stringSize should be more idiomatic

Ivan Gerasimov ivan.gerasimov at oracle.com
Mon Nov 23 13:34:51 UTC 2015


> What sanity checks you had in mind? Because
> java/lang/Integer/IntegerDecode.java fails, and it fails *in compiler*.
Oh, how embarrassing!  Overlooked a sign :(
I had tested it only with new lang/Integer/ToString.java, in a hope it 
gives enough coverage for sanity, but I was wrong.

> I tried to see what might be wrong, but failed.
>
> The mere fact the debugging is hard for this code tells me that we are
> much better off having cleaner code that handles MIN_VALUE separately.
> If you still want MIN_VALUE to go away, then file a follow up, and try
> to disentangle those weird failures ;)

I don't think the change is too complicated.
It just moves all the calculations into negative area, so we only need 
to be careful with signs :)

With this fixed patch:
http://cr.openjdk.java.net/~igerasim/8136500/8136500-addition-1.patch
all tests from test/lang pass.

Would you give it another chance?

> Also, with this kind of change, AbstractStringBuilders would *still*
> have the MIN_VALUE checks, *plus* all the additional handling logic in
> stringSize and getChars, that will run for positive values.
Incorporating this into ASB will result in removal of one if-statement 
altogether, so it seems to be a clear win.


Of course, it can be done later as a separate fix.

Sincerely yours,
Ivan

>   You can
> explore removing that as well, but it makes sense only after Indify
> String Concat lands and brings its own stringSize/getChar usages (and
> tests!) on the table.
>
>
>> Do you think it's worth it to reorganize the code this way?
> No, I think the perfect is the enemy of done here.
>
> Thanks,
> -Aleksey
>




More information about the core-libs-dev mailing list