Fix uint32 bug

[racketprogram]

That float verification operation is wrong when the value larger than int32max

[racketprogram]

What is that?

./node_modules/.bin/zuul -- ./test/[12]?.*.js
/home/travis/build/kawanet/msgpack-lite/node_modules/zuul/node_modules/wd/node_modules/request/node_modules/tough-cookie/node_modules/ip-regex/index.js:3
const v4 = '(?:25[0-5]|2[0-4][0-9]|1[0-9][0-9]|[1-9][0-9]|[0-9])(?:\\.(?:25[0-
^^^^^
SyntaxError: Use of const in strict mode.
    at exports.runInThisContext (vm.js:73:16)
    at Module._compile (module.js:443:25)
    at Object.Module._extensions..js (module.js:478:10)
    at Module.load (module.js:355:32)
    at Function.Module._load (module.js:310:12)
    at Module.require (module.js:365:17)
    at require (module.js:384:17)
    at Object.<anonymous> (/home/travis/build/kawanet/msgpack-lite/node_modules/zuul/node_modules/wd/node_modules/request/node_modules/tough-cookie/lib/cookie.js:34:15)
    at Module._compile (module.js:460:26)
    at Object.Module._extensions..js (module.js:478:10)

[racketprogram]

Hello, anybody here?

[marco6]

This should be leaved as is: it has the same semantics, but there are performance consideration to take into account. See https://jsperf.com/math-floor-alternatives

[marco6]

I think this, together with line 74 are the most important. All the others should be reversed.

[marco6]

I reviewed your code and it seems exactly what I need. @kawanet colud you please consider relasing a new version with this bugfix?
I can see from npm that this package has almost 400k downloads weekly. Would you consider letting someone else mantain it?
Otherwise, should we consider this package as dead and look for something else?

[racketprogram]

Oh, you are so kind, I can reverse that and keep bug fixed.

[kawanet]

Thank you guys for sending the PR and reviewing it!
@marco6 is right that someone wanted to keep maintaining this.

I have loved value | 0 thing which tells JIT to use integer type instead of double type as @marco6 mentioned. test/10.encode.js shows me that the author intends not to support the value over 0x7FFFFFFF as uint32 encoding.

  it("cc-cf: uint 8/16/32/64", function() {
    assert.deepEqual(toArray(msgpack.encode(0xFF, options)), [0xcc, 0xFF]);
    assert.deepEqual(toArray(msgpack.encode(0xFFFF, options)), [0xcd, 0xFF, 0xFF]);
    assert.deepEqual(toArray(msgpack.encode(0x7FFFFFFF, options)), [0xce, 0x7F, 0xFF, 0xFF, 0xFF]);
  });

The author seems to have decided for 0x80000000 to fallback to float64 encoding instead of uint32 encoding. If it support value over 0x7FFFFFFF, The test should have the other edge cases like below:

    assert.deepEqual(toArray(msgpack.encode(0x80000000, options)), [0xce, 0x80, 0x00, 0x00, 0x00]);
    assert.deepEqual(toArray(msgpack.encode(0xFFFFFFFF, options)), [0xce, 0xFF, 0xFF, 0xFF, 0xFF]);

I'm not sure that the performance difference still remains between value | 0 and value % 1 on the current version of V8, though. Anyway, uint32 encoding for 0x80000000 should be better.

[kawanet]

See https://jsperf.com/math-floor-alternatives

It's interesting that now Math.floor(value) itself could run at the same speed of value | 0.
Yet another smaller change below passes tests for 0x80000000 and 0xFFFFFFFF.

  function number(encoder, value) {
    var ivalue = Math.floor(value);
    var type;
    if (value !== ivalue || value < -0x80000000 || 0xffffffff < value) {

By the way, @racketprogram's code supports uint64 and int64 encodings! Cool!

      type = (value <= 0xFF) ? 0xcc : (value <= 0xFFFF) ? 0xcd : (value <= 0xFFFFFFFF) ? 0xce : 0xcf;
      type = (-0x80 <= value) ? 0xd0 : (-0x8000 <= value) ? 0xd1 : (-0x80000000 <= value) ? 0xd2 : 0xd3;

Let me check it would not have a rounding error on round trip encoding/decoding as JavaScript's number (double) only has 53bit precision.

[kawanet]

It's interesting that now Math.floor(value) itself could run at the same speed of value | 0.

Chrome is too genius as above.
Safari is still 30% slower on Math.floor(value).
Anyway, Node.js uses V8.
We rarely need to keep value | 0 hacks with Node.js any more?

[racketprogram]

We still can not use a positive integer to be an int type in javascript, because javascript not have type hint.

[kawanet]

We still can not use a positive integer to be an int type in javascript, because javascript not have type hint.

Right. ECMA-262 does not have a type hint in the spec.
V8 however handles int32 value internally for its optimizations.
https://books.google.co.jp/books?id=LvNFDwAAQBAJ&pg=PA30&dq=v8+uses+32-bit+numbers

I have loved such hacks of int32 value which may help V8 run fast.
However, the most recent version of V8 would be more clever than before.
Without the hacks, it may run enough fast on the recent V8.

[racketprogram]

I have a little confusing about the direction of that PR, Could you give me some comments or I can just wait a moment.
By the way, I don't know why CI testing was not triggered when I push a commit about testing.
Thanks a lot.

[racketprogram]

@kawanet Hello ?