Skip to content

Commit

Permalink
buffer: refactor all read/write functions
Browse files Browse the repository at this point in the history
There are a lot of changes in this commit:

1) Remove the `noAssert` argument from all read and write functions.
2) Improve the performance of all read floating point functions
   significantly. This is done by switching to TypedArrays as the
   write floating point write functions.
3) No implicit type coercion for offset and byteLength anymore.
4) Adds a lot of tests.
5) Moves the read and write functions to the internal buffer file
   to split the files in smaller chunks.
6) Reworked a lot of existing tests.
7) Improve the performane of all all read write functions by using
   a faster input validation and by improving function logic.
8) Significantly improved the performance of all read int functions.
   This is done by using a implementation without a loop.
9) Improved error handling.
10) Rename test file to use the correct subsystem.

PR-URL: nodejs#18395
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Anatoli Papirovski <apapirovski@mac.com>
Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl>
  • Loading branch information
BridgeAR authored and MayaLekova committed May 8, 2018
1 parent 6a3ffc7 commit dfb0f23
Show file tree
Hide file tree
Showing 21 changed files with 2,015 additions and 1,839 deletions.
269 changes: 144 additions & 125 deletions doc/api/buffer.md

Large diffs are not rendered by default.

555 changes: 4 additions & 551 deletions lib/buffer.js

Large diffs are not rendered by default.

786 changes: 785 additions & 1 deletion lib/internal/buffer.js

Large diffs are not rendered by default.

7 changes: 0 additions & 7 deletions test/parallel/test-buffer-arraybuffer.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,6 @@ assert.throws(function() {
Buffer.from(new AB());
}, TypeError);

// write{Double,Float}{LE,BE} with noAssert should not crash, cf. #3766
const b = Buffer.allocUnsafe(1);
b.writeFloatLE(11.11, 0, true);
b.writeFloatBE(11.11, 0, true);
b.writeDoubleLE(11.11, 0, true);
b.writeDoubleBE(11.11, 0, true);

// Test the byteOffset and length arguments
{
const ab = new Uint8Array(5);
Expand Down
25 changes: 2 additions & 23 deletions test/parallel/test-buffer-read.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,11 @@ const assert = require('assert');
const buf = Buffer.from([0xa4, 0xfd, 0x48, 0xea, 0xcf, 0xff, 0xd9, 0x01, 0xde]);

function read(buff, funx, args, expected) {

assert.strictEqual(buff[funx](...args), expected);
common.expectsError(
() => buff[funx](-1, args[1]),
{
code: 'ERR_INDEX_OUT_OF_RANGE'
}
{ code: 'ERR_OUT_OF_RANGE' }
);

assert.strictEqual(buff[funx](...args, true), expected);
}

// testing basic functionality of readDoubleBE() and readDoubleLE()
Expand Down Expand Up @@ -123,7 +118,7 @@ assert.throws(() => Buffer.allocUnsafe(8).readFloatLE(-1), RangeError);
(0xFFFFFFFF >> (32 - bits)));
});

// test for common read(U)IntLE/BE
// Test for common read(U)IntLE/BE
{
const buf = Buffer.from([0x01, 0x02, 0x03, 0x04, 0x05, 0x06]);

Expand All @@ -144,19 +139,3 @@ assert.throws(() => Buffer.allocUnsafe(8).readFloatLE(-1), RangeError);
assert.strictEqual(buf.readIntLE(0, 6), 0x060504030201);
assert.strictEqual(buf.readIntBE(0, 6), 0x010203040506);
}

// test for byteLength parameter not between 1 and 6 (inclusive)
common.expectsError(() => { buf.readIntLE(1); }, { code: 'ERR_OUT_OF_RANGE' });
common.expectsError(() => { buf.readIntLE(1, 'string'); },
{ code: 'ERR_OUT_OF_RANGE' });
common.expectsError(() => { buf.readIntLE(1, 0); },
{ code: 'ERR_OUT_OF_RANGE' });
common.expectsError(() => { buf.readIntLE(1, 7); },
{ code: 'ERR_OUT_OF_RANGE' });
common.expectsError(() => { buf.readIntBE(1); }, { code: 'ERR_OUT_OF_RANGE' });
common.expectsError(() => { buf.readIntBE(1, 'string'); },
{ code: 'ERR_OUT_OF_RANGE' });
common.expectsError(() => { buf.readIntBE(1, 0); },
{ code: 'ERR_OUT_OF_RANGE' });
common.expectsError(() => { buf.readIntBE(1, 7); },
{ code: 'ERR_OUT_OF_RANGE' });
138 changes: 138 additions & 0 deletions test/parallel/test-buffer-readdouble.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
'use strict';

require('../common');
const assert = require('assert');

// Test (64 bit) double
const buffer = Buffer.allocUnsafe(8);

buffer[0] = 0x55;
buffer[1] = 0x55;
buffer[2] = 0x55;
buffer[3] = 0x55;
buffer[4] = 0x55;
buffer[5] = 0x55;
buffer[6] = 0xd5;
buffer[7] = 0x3f;
assert.strictEqual(buffer.readDoubleBE(0), 1.1945305291680097e+103);
assert.strictEqual(buffer.readDoubleLE(0), 0.3333333333333333);

buffer[0] = 1;
buffer[1] = 0;
buffer[2] = 0;
buffer[3] = 0;
buffer[4] = 0;
buffer[5] = 0;
buffer[6] = 0xf0;
buffer[7] = 0x3f;
assert.strictEqual(buffer.readDoubleBE(0), 7.291122019655968e-304);
assert.strictEqual(buffer.readDoubleLE(0), 1.0000000000000002);

buffer[0] = 2;
assert.strictEqual(buffer.readDoubleBE(0), 4.778309726801735e-299);
assert.strictEqual(buffer.readDoubleLE(0), 1.0000000000000004);

buffer[0] = 1;
buffer[6] = 0;
buffer[7] = 0;
assert.strictEqual(buffer.readDoubleBE(0), 7.291122019556398e-304);
assert.strictEqual(buffer.readDoubleLE(0), 5e-324);

buffer[0] = 0xff;
buffer[1] = 0xff;
buffer[2] = 0xff;
buffer[3] = 0xff;
buffer[4] = 0xff;
buffer[5] = 0xff;
buffer[6] = 0x0f;
buffer[7] = 0x00;
assert.ok(Number.isNaN(buffer.readDoubleBE(0)));
assert.strictEqual(buffer.readDoubleLE(0), 2.225073858507201e-308);

buffer[6] = 0xef;
buffer[7] = 0x7f;
assert.ok(Number.isNaN(buffer.readDoubleBE(0)));
assert.strictEqual(buffer.readDoubleLE(0), 1.7976931348623157e+308);

buffer[0] = 0;
buffer[1] = 0;
buffer[2] = 0;
buffer[3] = 0;
buffer[4] = 0;
buffer[5] = 0;
buffer[6] = 0xf0;
buffer[7] = 0x3f;
assert.strictEqual(buffer.readDoubleBE(0), 3.03865e-319);
assert.strictEqual(buffer.readDoubleLE(0), 1);

buffer[6] = 0;
buffer[7] = 0x40;
assert.strictEqual(buffer.readDoubleBE(0), 3.16e-322);
assert.strictEqual(buffer.readDoubleLE(0), 2);

buffer[7] = 0xc0;
assert.strictEqual(buffer.readDoubleBE(0), 9.5e-322);
assert.strictEqual(buffer.readDoubleLE(0), -2);

buffer[6] = 0x10;
buffer[7] = 0;
assert.strictEqual(buffer.readDoubleBE(0), 2.0237e-320);
assert.strictEqual(buffer.readDoubleLE(0), 2.2250738585072014e-308);

buffer[6] = 0;
assert.strictEqual(buffer.readDoubleBE(0), 0);
assert.strictEqual(buffer.readDoubleLE(0), 0);
assert.ok(1 / buffer.readDoubleLE(0) >= 0);

buffer[7] = 0x80;
assert.strictEqual(buffer.readDoubleBE(0), 6.3e-322);
assert.strictEqual(buffer.readDoubleLE(0), -0);
assert.ok(1 / buffer.readDoubleLE(0) < 0);

buffer[6] = 0xf0;
buffer[7] = 0x7f;
assert.strictEqual(buffer.readDoubleBE(0), 3.0418e-319);
assert.strictEqual(buffer.readDoubleLE(0), Infinity);

buffer[7] = 0xff;
assert.strictEqual(buffer.readDoubleBE(0), 3.04814e-319);
assert.strictEqual(buffer.readDoubleLE(0), -Infinity);

['readDoubleLE', 'readDoubleBE'].forEach((fn) => {
['', '0', null, undefined, {}, [], () => {}, true, false].forEach((off) => {
assert.throws(
() => buffer[fn](off),
{ code: 'ERR_INVALID_ARG_TYPE' }
);
});

[Infinity, -1, 1].forEach((offset) => {
assert.throws(
() => buffer[fn](offset),
{
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError [ERR_OUT_OF_RANGE]',
message: 'The value of "offset" is out of range. ' +
`It must be >= 0 and <= 0. Received ${offset}`
});
});

assert.throws(
() => Buffer.alloc(1)[fn](1),
{
code: 'ERR_BUFFER_OUT_OF_BOUNDS',
name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
message: 'Attempt to write outside buffer bounds'
});

[NaN, 1.01].forEach((offset) => {
assert.throws(
() => buffer[fn](offset),
{
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError [ERR_OUT_OF_RANGE]',
message: 'The value of "offset" is out of range. ' +
`It must be an integer. Received ${offset}`
});
});
});
101 changes: 101 additions & 0 deletions test/parallel/test-buffer-readfloat.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
'use strict';

require('../common');
const assert = require('assert');

// Test 32 bit float
const buffer = Buffer.alloc(4);

buffer[0] = 0;
buffer[1] = 0;
buffer[2] = 0x80;
buffer[3] = 0x3f;
assert.strictEqual(buffer.readFloatBE(0), 4.600602988224807e-41);
assert.strictEqual(buffer.readFloatLE(0), 1);

buffer[0] = 0;
buffer[1] = 0;
buffer[2] = 0;
buffer[3] = 0xc0;
assert.strictEqual(buffer.readFloatBE(0), 2.6904930515036488e-43);
assert.strictEqual(buffer.readFloatLE(0), -2);

buffer[0] = 0xff;
buffer[1] = 0xff;
buffer[2] = 0x7f;
buffer[3] = 0x7f;
assert.ok(Number.isNaN(buffer.readFloatBE(0)));
assert.strictEqual(buffer.readFloatLE(0), 3.4028234663852886e+38);

buffer[0] = 0xab;
buffer[1] = 0xaa;
buffer[2] = 0xaa;
buffer[3] = 0x3e;
assert.strictEqual(buffer.readFloatBE(0), -1.2126478207002966e-12);
assert.strictEqual(buffer.readFloatLE(0), 0.3333333432674408);

buffer[0] = 0;
buffer[1] = 0;
buffer[2] = 0;
buffer[3] = 0;
assert.strictEqual(buffer.readFloatBE(0), 0);
assert.strictEqual(buffer.readFloatLE(0), 0);
assert.ok(1 / buffer.readFloatLE(0) >= 0);

buffer[3] = 0x80;
assert.strictEqual(buffer.readFloatBE(0), 1.793662034335766e-43);
assert.strictEqual(buffer.readFloatLE(0), -0);
assert.ok(1 / buffer.readFloatLE(0) < 0);

buffer[0] = 0;
buffer[1] = 0;
buffer[2] = 0x80;
buffer[3] = 0x7f;
assert.strictEqual(buffer.readFloatBE(0), 4.609571298396486e-41);
assert.strictEqual(buffer.readFloatLE(0), Infinity);

buffer[0] = 0;
buffer[1] = 0;
buffer[2] = 0x80;
buffer[3] = 0xff;
assert.strictEqual(buffer.readFloatBE(0), 4.627507918739843e-41);
assert.strictEqual(buffer.readFloatLE(0), -Infinity);

['readFloatLE', 'readFloatBE'].forEach((fn) => {
['', '0', null, undefined, {}, [], () => {}, true, false].forEach((off) => {
assert.throws(
() => buffer[fn](off),
{ code: 'ERR_INVALID_ARG_TYPE' }
);
});

[Infinity, -1, 1].forEach((offset) => {
assert.throws(
() => buffer[fn](offset),
{
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError [ERR_OUT_OF_RANGE]',
message: 'The value of "offset" is out of range. ' +
`It must be >= 0 and <= 0. Received ${offset}`
});
});

assert.throws(
() => Buffer.alloc(1)[fn](1),
{
code: 'ERR_BUFFER_OUT_OF_BOUNDS',
name: 'RangeError [ERR_BUFFER_OUT_OF_BOUNDS]',
message: 'Attempt to write outside buffer bounds'
});

[NaN, 1.01].forEach((offset) => {
assert.throws(
() => buffer[fn](offset),
{
code: 'ERR_OUT_OF_RANGE',
name: 'RangeError [ERR_OUT_OF_RANGE]',
message: 'The value of "offset" is out of range. ' +
`It must be an integer. Received ${offset}`
});
});
});
Loading

0 comments on commit dfb0f23

Please sign in to comment.