Review request for JDK-8012334: ToUint32, ToInt32, and ToUint16 don't conform to spec

Attila Szegedi attila.szegedi at oracle.com
Wed Apr 24 03:51:40 PDT 2013


+1 from me as well. You did as well as it could be done to be both correct and performant.

On Apr 24, 2013, at 11:06 AM, Hannes Wallnoefer <hannes.wallnoefer at oracle.com> wrote:

> On Marcus' request I also replaced hardcoded instances of 0xffff_ffffL with JSType.MAX_UINT throughout the code base. Already got +1 from Marcus, I need one more review please.
> 
> http://cr.openjdk.java.net/~hannesw/8012334/webrev.02/
> 
> Hannes
> 
> Am 2013-04-24 06:33, schrieb Marcus Lagergren:
>> OK. We are introducing method specialization based on call sites and we ARE introducing strength reduction based on rage analysis, so I guess this is +1 for me.
>> 
>> On Apr 24, 2013, at 12:26 AM, Hannes Wallnoefer <hannes.wallnoefer at oracle.com> wrote:
>> 
>>> The microbenchmark goes from about 250 millis to about 380 for most numbers. But I guess we'll have to bite the bullet. Also V8 takes around 380 milliseconds when it actually converts from double (much faster when it's able to optimize to ints) so I guess it's just the time it takes.
>>> 
>>> I also did a few octane runs and couldn't detect any regression there.
>>> 
>>> Hannes
>>> 
>>> Am 2013-04-23 20:51, schrieb Marcus Lagergren:
>>>> How much is a bit and what did you measure?
>>>> 
>>>> On Apr 23, 2013, at 8:33 PM, Hannes Wallnoefer <hannes.wallnoefer at oracle.com> wrote:
>>>> 
>>>>> Please review webrev for JDK-8012334: ToUint32, ToInt32, and ToUint16 don't conform to spec:
>>>>> 
>>>>> http://cr.openjdk.java.net/~hannesw/8012334/
>>>>> 
>>>>> The code path for small numbers is now a little bit slower but not much. The code path for large numbers  that don't fit in 32 bits is quite a bit slower, but it should be correct now. The patch also contains a microbenchmark for ToInt32 (signed right shift) and ToUint32 (unsigned right shift).
>>>>> 
>>>>> Hannes
> 



More information about the nashorn-dev mailing list