Convert serialized values to ints if they are whole and less than 53 bits.

Convert serialized values to ints if they are whole and less than 53 bits.


BUG=
R=sra@google.com

Committed: https://src.chromium.org/viewvc/multivm?view=rev&revision=1454

[Emily Fortuna, 2013-10-17 21:20:49]

BUG = https://code.google.com/p/dart/issues/detail?id=11448

Dartium change corresponding to: https://codereview.chromium.org/26308004/
change in Dart codebase

Suggestions for a better way to do this are welcome.

[sra1, 2013-10-17 21:40:31]

https://codereview.chromium.org/27767003/diff/1/Source/bindings/dart/DartUtil...
File Source/bindings/dart/DartUtilities.cpp (right):

https://codereview.chromium.org/27767003/diff/1/Source/bindings/dart/DartUtil...
Source/bindings/dart/DartUtilities.cpp:288: if (value == intValue &&
abs(intValue) <= pow(2, 53))
I would avoid calling pow and define a constant
kMaxExactDouble = 9007199254740991;  // 2^53-1

2^53 is the first integer that has who mathematical integers folded onto it,
hence the -1 above.  (Math.pow(2,53) == Math.pow(2,53)+1  in the chrome
console).

First compare the range because that avoids problems with infinites and NaN
values.

if (value >= -kMaxExactDouble || value <= kMaxExactDouble) {
  // now see if there is a fraction part
}

[Emily Fortuna, 2013-10-17 22:42:51]

ptal

also found the constant defined elsewhere. :-P

https://codereview.chromium.org/27767003/diff/1/Source/bindings/dart/DartUtil...
File Source/bindings/dart/DartUtilities.cpp (right):

https://codereview.chromium.org/27767003/diff/1/Source/bindings/dart/DartUtil...
Source/bindings/dart/DartUtilities.cpp:288: if (value == intValue &&
abs(intValue) <= pow(2, 53))
On 2013/10/17 21:40:31, sra1 wrote:
> I would avoid calling pow and define a constant
> kMaxExactDouble = 9007199254740991;  // 2^53-1
> 
> 2^53 is the first integer that has who mathematical integers folded onto it,
> hence the -1 above.  (Math.pow(2,53) == Math.pow(2,53)+1  in the chrome
> console).
> 
> First compare the range because that avoids problems with infinites and NaN
> values.
> 
> if (value >= -kMaxExactDouble || value <= kMaxExactDouble) {
>   // now see if there is a fraction part
> }

Gotcha. Sorry I was going off of this:
http://ecma262-5.com/ELS5_HTML.htm#Section_8.5 "Note that all the positive and
negative integers whose magnitude is no greater than 253 are representable in
the Number type"

[Emily Fortuna, 2013-10-17 23:10:39]

take 3

[sra1, 2013-10-18 02:56:28]

lgtm

https://chromiumcodereview.appspot.com/27767003/diff/11001/Source/bindings/da...
File Source/bindings/dart/DartUtilities.cpp (right):

https://chromiumcodereview.appspot.com/27767003/diff/11001/Source/bindings/da...
Source/bindings/dart/DartUtilities.cpp:286: if (value <= kJSMaxInteger && value
>= -kJSMaxInteger) {
nit: This would be slightly more readable:

if (-k <= value && value <= k) ...

[vsm, 2013-10-18 20:53:26]

https://chromiumcodereview.appspot.com/27767003/diff/11001/Source/bindings/da...
File Source/bindings/dart/V8Converter.cpp (right):

https://chromiumcodereview.appspot.com/27767003/diff/11001/Source/bindings/da...
Source/bindings/dart/V8Converter.cpp:178: return
Dart_NewDouble(value->NumberValue());
Why not call DartUtilities:numberToDart(...) here and eliminate the earlier
clause on line 173?

[Emily Fortuna, 2013-10-18 21:24:05]

https://chromiumcodereview.appspot.com/27767003/diff/11001/Source/bindings/da...
File Source/bindings/dart/V8Converter.cpp (right):

https://chromiumcodereview.appspot.com/27767003/diff/11001/Source/bindings/da...
Source/bindings/dart/V8Converter.cpp:178: return
Dart_NewDouble(value->NumberValue());
On 2013/10/18 20:53:26, vsm wrote:
> Why not call DartUtilities:numberToDart(...) here and eliminate the earlier
> clause on line 173?

Good point. Fixed.

[Emily Fortuna, 2013-10-18 21:28:39]

Committed patchset #5 manually as r1454.