I tried to add in the extra test, but messed up I think because it did not copy over the original tests in the revision (even though I was 'adding' a new test). I have not used jsperf before, so probably did something stupid. On the face of it, it certainly does look like the byte level stuff is faster.
On Sun, Jun 24, 2018 at 10:32 AM Greg Dove <[email protected]> wrote: > > Harbs, for the typed array test, try using the ArrayBuffer in the > constructor instead of the Uint8Array, I think you might see a difference > here. > > > On Sun, Jun 24, 2018 at 10:09 AM Harbs <[email protected]> wrote: > >> I created a jsperf which compares different read methods, and the one I >> changed it to seems fastest by far: >> https://jsperf.com/typedarray-read-performance/1 < >> https://jsperf.com/typedarray-read-performance/1> >> >> The only optimization which might give slight gains is to use shift >> operators instead of multiplication. Enabling the --enable-benchmarking >> flag in Chrome seemed to indicate that there’s up to a 10% performance >> increase of the shift operators over multiplication. Logically, shift >> operators should be cheaper. In Safari, the shift operator gave a >> performance boost of 3 times the multiplication. >> >> I only tested 16 bit uints. It might be worthwhile adding ints and 32 bit. >> >> Thanks, >> Harbs >> >> > On Jun 22, 2018, at 8:58 AM, Alex Harui <[email protected]> >> wrote: >> > >> > It is definitely true that I don't know how expensive instantiation of >> small instances is in the browser. Might even be different in different >> browsers. There should also be function call overhead in calling the >> constructor as well. On the other hand, maybe using the typed arrays helps >> with the browser's type inferencing and there is some advantage there that >> also assumes the equivalent of word/long alignments. >> > >> > If TypeArrays really aren't that expensive to instantiate, it may be >> that the way to handle floats/doubles is to copy bytes into a new >> BinaryData and convert that to the appropriate TypedArray. That should >> ensure that there won't be alignment issues. >> > >> > IMO, this discussion shows the importance of providing choices. You >> can have a much simpler variant that assumes certain endian and thus save a >> whole bunch of byte swapping. You might in fact have a faster >> implementation if you can guarantee/assume alignment. >> > >> > HTH, >> > -Alex >> > >> > On 6/21/18, 3:01 PM, "Greg Dove" <[email protected]> wrote: >> > >> > ' If you have the time to resolve these remaining issues, that would >> be >> > awesome.' >> > >> > Happy to commit to doing this by mid-July if that works. I do have >> downtime >> > in July that I can use for Royale, but I also need to set up for >> Royale >> > development again because I have changed machines. >> > >> > >> > On Fri, Jun 22, 2018 at 9:13 AM Harbs <[email protected]> wrote: >> > >> >> Cool. >> >> >> >> FYI there are still some issues to resolve: >> >> >> >> 1. readFloat and readDouble should have the same issues as the various >> int >> >> read methods. I did not fix that yet because calculating floats and >> doubles >> >> is harder than ints. I’m not sure of the best way to handle that. One >> way >> >> is to create a new temporary ArrayBuffer with only the necessary bytes >> and >> >> read that. Another would be to use DataViews for floats and doubles. A >> >> third would be to actually do the math for floating point numbers… >> >> >> >> 2. I did not fix any of the write methods. They should have the same >> issue >> >> as the read methods. >> >> >> >> If you have the time to resolve these remaining issues, that would be >> >> awesome. :-) >> >> >> >> Harbs >> >> >> >>> On Jun 21, 2018, at 11:55 PM, Greg Dove <[email protected]> wrote: >> >>> >> >>> Thanks Harbs - that short explanation makes it very clear. Thanks for >> >>> fixing my broken implementation :). I am guilty of not covering 'real >> >>> world' combinations in the tests, which was what I used to guide >> >>> development when I was working on this originally. >> >>> >> >>> I am pretty sure I did have also have an interim implementation doing >> the >> >>> bytewise reads/writes and dealing with endian-ness branching along the >> >> way, >> >>> then I switched to using the typed arrays when I saw a performance >> boost. >> >>> I wish I had written something like your new test before I did that >> :). >> >>> I did a quick search for this and read up on the issue about the >> offset. >> >> I >> >>> guess that also partly explains why the typed array performance is >> >> 'faster' >> >>> (possibly due to the alignment I mean). And I did not do memory >> >> profiling, >> >>> so maybe Alex is right - there could also have been a massive GC dump >> >> after >> >>> my large loop tests, something that I was not seeing because I was >> only >> >>> focused on speed at the time. >> >>> >> >>> I had to add the 'sysEndian' flag approach internally when I switched >> to >> >>> using the typed arrays (and only reordering the result if it was >> >> different >> >>> to the 'native' endian used in the typed arrays). If everything is >> based >> >> on >> >>> bytewise approach now, that flag should no longer be needed I think >> (you >> >>> may already have done that, I did not look at your changes in detail >> yet >> >> - >> >>> will check them out this weekend). >> >>> >> >>> Obviously having this working correctly comes first, but with a >> >>> comprehensive set of tests, we do have the ability to more easily >> respond >> >>> to changes in the browser engines over time and update the >> implementation >> >>> if, for example, DataView becomes faster across the board. I did see >> it >> >>> working faster in one of the browsers I tested in, but I am not >> confident >> >>> that I kept a good record of all this... >> >>> >> >>> I will look in more detail this weekend, but for now it sounds like >> you >> >>> have resolved the issue - well done! I hope I can get into some more >> work >> >>> on Royale again soon. >> >>> >> >>> >> >>> >> >>> On Thu, 21 Jun 2018, 21:04 Harbs, <[email protected]> wrote: >> >>> >> >>>> I just added a unit test for this. >> >>>> >> >>>> The test will fail with a RTE using the old implementation. >> >>>> >> >>>> Explanation: After reading the first byte, the position is 1 which >> >> doesn’t >> >>>> align with a 16IntArray. >> >>>> >> >>>> Thanks, >> >>>> Harbs >> >>>> >> >>>>> On Jun 21, 2018, at 1:48 AM, Greg Dove <[email protected]> wrote: >> >>>>> >> >>>>> I am not sure what the int16/int-aligned issue is yet (I have not >> read >> >>>> back >> >>>>> through all the relevant history) but if we can set up a unit test >> for >> >>>> it, >> >>>>> this stuff all becomes much clearer I think. >> >>>> >> >>>> >> >> >> >> >> > >> > >> >>
