I'm still pondering if there are issues. More related to lots of integer math like is used in MD5.
FWIW, I'm not clear that ">>> 0" actually results in a call to the CPU's shift operator or even the Browser's shift code. It might just be used for type inferencing. My 2 cents, -Alex On 6/25/18, 10:18 AM, "Harbs" <[email protected]> wrote: I already switched to using the unsigned shift operator. I’m more trying to understand if there are issues I’m not getting. I wonder if all the uint read methods should be using the unsigned shift operator to ensure the number type is an unsigned integer before returning the value, but I’m not sure it really has an effect on the bits. Adding an extra operation which doesn’t do anything is silly. If you have any ideas for tests which could be effected by bits that are set wrong somehow, I’d love to add those… Regarding performance, it does seem like shift operators are for the most part more efficient, but it depends on the browser. Safari had an improvement of between 2 and three times for 16 bit conversions. Chrome was actually a bit slower with short operators until the enable-profiling flag was enabled. Then shift operators where about 50% faster. Of course, <<24 >>0 is two operations instead of one, so I’m guessing that performance gains over multiplication will be less (but I could be wrong). Thanks, Harbs > On Jun 25, 2018, at 8:08 PM, Alex Harui <[email protected]> wrote: > > My main concern is having Royale "do the right thing". I believe that some of our migrating users have code that uses bit shifting and if it doesn't work correctly on JS it would seem a bit ugly to recommend that they also use multiplication. If ">>> 0" works, that is something the compiler could automatically generate so users don't have to touch migrating code that bit shifts unsigned integers. > > If it is really important to you to use multiplication in BinaryData, fine, but I think folks will eventually notice and wonder why. > > I don't know how the browsers actually work these days or will work in the future, but I'm more likely to bet that they will optimize bit shifting if we can help them infer types. I'm not sure they can as easily infer types from multiplication as from actual bit shifting. > > Finally, way back in the day, bit shifting was much faster than floating point multiplication, but you never know how much overhead there is in the browser runtime. > > My 2 cents, > -Alex > > On 6/25/18, 12:57 AM, "Harbs" <[email protected] <mailto:[email protected]>> wrote: > > I didn’t find that >>> 0 trick before. Interesting. > > I guess I could switch the calculation to (val << 24) >>> 0 which seems to give the correct results, but I’m really not sure what your concern is with the multiplication. All we’re doing is getting the numeric value of the last byte. I don’t see how we could get a different result after adding the four byte values together. > > Thanks, > Harbs > >> On Jun 25, 2018, at 10:46 AM, Alex Harui <[email protected] <mailto:[email protected]>> wrote: >> >> Here's an article that might help. https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstackoverflow.com%2Fquestions%2F6798111%2Fbitwise-operations-on-32-bit-unsigned-ints%3Frq%3D1&data=02%7C01%7Caharui%40adobe.com%7Cc1b81673ea22436323fe08d5da7157df%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636655102772966878&sdata=NrDyYfPqE2PKnzCHOZlyOPppaLYQukFzBTq9UeRvVhg%3D&reserved=0 <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstackoverflow.com%2Fquestions%2F6798111%2Fbitwise-operations-on-32-bit-unsigned-ints%3Frq%3D1&data=02%7C01%7Caharui%40adobe.com%7Cc1b81673ea22436323fe08d5da7157df%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636655102772966878&sdata=NrDyYfPqE2PKnzCHOZlyOPppaLYQukFzBTq9UeRvVhg%3D&reserved=0> >> >> It's great that you have it working with multiply, but my main concern is if existing code being migrated uses shift operators and whether JS will produce different results than SWF. Maybe we need to do the >>> 0 trick mentioned in this article. >> >> HTH, >> -Alex >> >> On 6/24/18, 11:49 PM, "Harbs" <[email protected]> wrote: >> >> readUnsignedInt() was returning the wrong value when the last bit was 1 (i.e. a negative int instead of a large uint). Using multiplication on the last byte instead of bit shifting fixed the problem. It works because multiplication will always give the same correct positive value. I think that’s the simplest way to resolve this. >> >> Thanks, >> Harbs >> >>> On Jun 25, 2018, at 9:32 AM, Alex Harui <[email protected]> wrote: >>> >>> Yeah, that article is addressing what I meant by "out of range" or an "signed int read". What method is this that you are changing? If this is all unsigned, then the issue may not be the shifting, but rather the values being tested or the final conversion of the bits to the returned value. >>> >>> IOW, an unsigned int can just plain start as 0xFF000000. And if the browser is going to convert that to a signed int (actually, a float, I think), then we have to make sure we find the right way to convert that value as a uint. It might require stuffing a DataView or even using parseFloat. Not sure. >>> >>> My 2 cents, >>> -Alex >>> >>> On 6/24/18, 11:11 PM, "Harbs" <[email protected]> wrote: >>> >>> It seemed strange to me too. I found this: >>> https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstackoverflow.com%2Fquestions%2F18034974%2Fwhy-in-javascript-expression-255-24-is-a-negative-number&data=02%7C01%7Caharui%40adobe.com%7C5d2e791d3ff0427b519508d5da6282f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636655038993975700&sdata=jXJWpzL16YFtQt5AMqVWk6rj%2BYUYCSDI8OTngaZrcYw%3D&reserved=0 <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fstackoverflow.com%2Fquestions%2F18034974%2Fwhy-in-javascript-expression-255-24-is-a-negative-number&data=02%7C01%7Caharui%40adobe.com%7C5d2e791d3ff0427b519508d5da6282f9%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636655038993975700&sdata=jXJWpzL16YFtQt5AMqVWk6rj%2BYUYCSDI8OTngaZrcYw%3D&reserved=0> >>> >>>> On Jun 25, 2018, at 9:07 AM, Alex Harui <[email protected]> wrote: >>>> >>>> FWIW, this does not make sense. Shifting to the left shouldn't cause sign-bit extending. I suppose it could shift a 1 into the sign bit, but that implies a signed int read, or the data was out of range. >>>> >>>> Of course, I could be wrong... >>>> -Alex >>>> >>>> On 6/24/18, 12:25 PM, "[email protected] <mailto:[email protected]>" <[email protected] <mailto:[email protected]>> wrote: >>>> >>>> This is an automated email from the ASF dual-hosted git repository. >>>> >>>> harbs pushed a commit to branch develop >>>> in repository https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-asjs.git&data=02%7C01%7Caharui%40adobe.com%7Ce941b0896db44fa2127c08d5da08491f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636654651499005568&sdata=h3WWynmXGQXTRqmzte46oprTRzZf0abvL7cfCEmSgZA%3D&reserved=0 <https://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgitbox.apache.org%2Frepos%2Fasf%2Froyale-asjs.git&data=02%7C01%7Caharui%40adobe.com%7Ce941b0896db44fa2127c08d5da08491f%7Cfa7b1b5a7b34438794aed2c178decee1%7C0%7C1%7C636654651499005568&sdata=h3WWynmXGQXTRqmzte46oprTRzZf0abvL7cfCEmSgZA%3D&reserved=0> >>>> >>>> >>>> The following commit(s) were added to refs/heads/develop by this push: >>>> new ce95546 Shifting 24 bits converted to negative int value >>>> ce95546 is described below >>>> >>>> commit ce95546395ade51c63ba9b8a9cff7c63477b8c4a >>>> Author: Harbs <[email protected] <mailto:[email protected]>> >>>> AuthorDate: Sun Jun 24 22:25:37 2018 +0300 >>>> >>>> Shifting 24 bits converted to negative int value >>>> --- >>>> .../Core/src/main/royale/org/apache/royale/utils/BinaryData.as | 4 ++-- >>>> 1 file changed, 2 insertions(+), 2 deletions(-) >>>> >>>> diff --git a/frameworks/projects/Core/src/main/royale/org/apache/royale/utils/BinaryData.as b/frameworks/projects/Core/src/main/royale/org/apache/royale/utils/BinaryData.as >>>> index c430369..fad4ea3 100644 >>>> --- a/frameworks/projects/Core/src/main/royale/org/apache/royale/utils/BinaryData.as >>>> +++ b/frameworks/projects/Core/src/main/royale/org/apache/royale/utils/BinaryData.as >>>> @@ -665,9 +665,9 @@ public class BinaryData implements IBinaryDataInput, IBinaryDataOutput >>>> { >>>> var arr:Uint8Array = getTypedArray(); >>>> if(endian == Endian.BIG_ENDIAN){ >>>> - return (arr[_position++] << 24) + (arr[_position++] << 16) + ( arr[_position++] << 8) + arr[_position++]; >>>> + return (arr[_position++] * 16777216) + (arr[_position++] << 16) + ( arr[_position++] << 8) + arr[_position++]; >>>> } else { >>>> - return arr[_position++] + ( arr[_position++] << 8) + (arr[_position++] << 16) + (arr[_position++] << 24) >>>> + return arr[_position++] + ( arr[_position++] << 8) + (arr[_position++] << 16) + (arr[_position++] * 16777216) >>>> } >>>> } >>>> }
