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 <greg.d...@gmail.com> 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, <harbs.li...@gmail.com> 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 <greg.d...@gmail.com> 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. >> >>
