On Oct 14, 2014, at 11:23 AM, Marcus Lagergren <[email protected]> wrote:
> * final missing in NativeFloat32Array and friends. Added. > * I want a unit test that checks that long is the correct representation for > uint32 array elements. uint16 should be fine as it's chars I guess. The getElementType() was previously only used by NativeArray.PopLinkLogic, so its value in Uint32ArrayData had no effect on anything. Uint32ArrayData getElem() and setElem() have both alway returned/taken long values, so long is the correct value. Which is to say, there's no test that could detect a difference from the behavior before this patch. > * There is whitespace missing between functions in NativeUint8Array. Added. > * Can you check that all unsigned typed arrays work for boundary conditions. Uint16 and Uint8 trivially fit into a 32-bit int, so they have to. Existing tests, specifically NASHORN-377 exercise all of these quite thoroughly, also lots of Octane benchmarks too (pdfjs and mandreel use a lot of Uint8, Uint16, Uint32, Float32). > > Otherwise, looking very good. Nicely spotted. > > /M > > On 13 Oct 2014, at 20:34, Attila Szegedi <[email protected]> wrote: > >> Please review JDK-8060242 at >> <http://cr.openjdk.java.net/~attila/8060242/webrev.00> for >> <https://bugs.openjdk.java.net/browse/JDK-8060242> >> >> The fix is mostly in the TypeEvaluator.java, the rest is just plumbing >> (ArrayBufferView has to be public so I can us it with instanceof; array data >> subclasses for buffer views need to support optimistic operations; >> Int/Long/Number/ObjectArrayData now inherit they getOptimisticType() from >> ContinuousArrayData; finally there was a genuine data type bug where >> NativeUint32Array reported its array element type to be int instead of long). >> >> Thanks, >> Attila. >
