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