Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(NODE-3630): remove float parser and test edge cases for Double #502

Merged
merged 12 commits into from
Jul 6, 2022
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 42 additions & 1 deletion etc/benchmarks/main.mjs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* eslint-disable @typescript-eslint/no-var-requires */
import { performance } from 'perf_hooks';
import bson from '../../lib/bson.js';
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
import { runner, systemInfo, getCurrentLocalBSON } from './lib_runner.mjs';

const iterations = 1_000_000;
Expand All @@ -9,7 +10,7 @@ console.log();

////////////////////////////////////////////////////////////////////////////////////////////////////
await runner({
skip: false,
skip: true,
name: 'deserialize({ oid, string }, { validation: { utf8: false } })',
iterations,
setup(libs) {
Expand Down Expand Up @@ -58,6 +59,46 @@ await runner({
}
});

await runner({
skip: true,
name: 'Double Serialization',
iterations,
run(i, bson) {
bson.lib.serialize({ d: 2.3 });
}
});

await runner({
skip: false,
name: 'Double Deserialization',
iterations,
setup(libs) {
const bson = getCurrentLocalBSON(libs);
return bson.lib.serialize({ d: 2.3 });
},
run(i, bson, serialized_double) {
bson.lib.deserialize(serialized_double);
}
});

await runner({
skip: false,
name: 'Many Doubles Deserialization',
iterations,
setup(libs) {
const bson = getCurrentLocalBSON(libs);
let doubles = Object.fromEntries(
Array.from({ length: 1000 }, i => {
return [`a_${i}`, 2.3];
})
);
return bson.lib.serialize(doubles);
},
run(i, bson, serialized_doubles) {
bson.lib.deserialize(serialized_doubles);
}
});

// End
console.log(
'Total time taken to benchmark:',
Expand Down
152 changes: 0 additions & 152 deletions src/float_parser.ts

This file was deleted.

5 changes: 3 additions & 2 deletions src/parser/deserializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ function deserializeObject(
let isPossibleDBRef = isArray ? false : null;

// While we have more left data left keep parsing
const dataview = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength);
while (!done) {
// Read the type
const elementType = buffer[index++];
Expand Down Expand Up @@ -263,10 +264,10 @@ function deserializeObject(
(buffer[index++] << 16) |
(buffer[index++] << 24);
} else if (elementType === constants.BSON_DATA_NUMBER && promoteValues === false) {
value = new Double(buffer.readDoubleLE(index));
value = new Double(dataview.getFloat64(index, true));
index = index + 8;
} else if (elementType === constants.BSON_DATA_NUMBER) {
value = buffer.readDoubleLE(index);
value = dataview.getFloat64(index, true);
index = index + 8;
} else if (elementType === constants.BSON_DATA_DATE) {
const lowBits =
Expand Down
13 changes: 10 additions & 3 deletions src/parser/serializer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import type { Double } from '../double';
import { ensureBuffer } from '../ensure_buffer';
import { BSONError, BSONTypeError } from '../error';
import { isBSONType } from '../extended_json';
import { writeIEEE754 } from '../float_parser';
import type { Int32 } from '../int_32';
import { Long } from '../long';
import { Map } from '../map';
Expand Down Expand Up @@ -79,6 +78,12 @@ function serializeString(
return index;
}

const SPACE_FOR_FLOAT64 = new Uint8Array(8);
const DV_FOR_FLOAT64 = new DataView(
SPACE_FOR_FLOAT64.buffer,
SPACE_FOR_FLOAT64.byteOffset,
SPACE_FOR_FLOAT64.byteLength
);
function serializeNumber(
buffer: Buffer,
key: string,
Expand Down Expand Up @@ -119,7 +124,8 @@ function serializeNumber(
index = index + numberOfWrittenBytes;
buffer[index++] = 0;
// Write float
writeIEEE754(buffer, value, index, 'little', 52, 8);
DV_FOR_FLOAT64.setFloat64(0, value, true);
buffer.set(SPACE_FOR_FLOAT64, index);
// Adjust index
index = index + 8;
}
Expand Down Expand Up @@ -487,7 +493,8 @@ function serializeDouble(
buffer[index++] = 0;

// Write float
writeIEEE754(buffer, value.value, index, 'little', 52, 8);
DV_FOR_FLOAT64.setFloat64(0, value.value, true);
buffer.set(SPACE_FOR_FLOAT64, index);

// Adjust index
index = index + 8;
Expand Down
4 changes: 2 additions & 2 deletions test/node/bson_corpus.spec.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

const Buffer = require('buffer').Buffer;
const BSON = require('../register-bson');
const { getNodeMajor, isBrowser } = require('./tools/utils');
const BSONError = BSON.BSONError;
const EJSON = BSON.EJSON;

Expand Down Expand Up @@ -121,8 +122,7 @@ describe('BSON Corpus', function () {
describe('valid-bson', function () {
for (const v of valid) {
it(v.description, function () {
if (v.description === 'NaN with payload') {
// TODO(NODE-3630): remove custom float parser so we can handle the NaN payload data
if (v.description === 'NaN with payload' && !isBrowser() && getNodeMajor() < 10) {
dariakp marked this conversation as resolved.
Show resolved Hide resolved
this.skip();
}

Expand Down
71 changes: 71 additions & 0 deletions test/node/double_tests.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
'use strict';

const BSON = require('../register-bson');
const { getNodeMajor, isBrowser } = require('./tools/utils');
const Double = BSON.Double;

describe('Double', function () {
Expand Down Expand Up @@ -35,4 +36,74 @@ describe('Double', function () {
});
}
});

describe('specialValues', () => {
dariakp marked this conversation as resolved.
Show resolved Hide resolved
function twiceSerialized(value) {
dariakp marked this conversation as resolved.
Show resolved Hide resolved
let serializedDouble = BSON.serialize({ d: value });
dariakp marked this conversation as resolved.
Show resolved Hide resolved
let deserializedDouble = BSON.deserialize(serializedDouble, { promoteValues: true });
let newVal = deserializedDouble.d;
return newVal;
}

it('inf', () => {
dariakp marked this conversation as resolved.
Show resolved Hide resolved
let value = Infinity;
let newVal = twiceSerialized(value);
expect(value).to.equal(newVal);
});

it('-inf', () => {
let value = -Infinity;
let newVal = twiceSerialized(value);
expect(value).to.equal(newVal);
});

it('NaN', () => {
let value = NaN;
let newVal = twiceSerialized(value);
expect(Number.isNaN(newVal)).to.equal(true);
});

it('NaN with payload', function () {
dariakp marked this conversation as resolved.
Show resolved Hide resolved
if (!isBrowser() && getNodeMajor() < 10) {
dariakp marked this conversation as resolved.
Show resolved Hide resolved
this.skip();
}
let buffer = Buffer.from('120000000000F87F', 'hex');

const dv = new DataView(buffer.buffer, buffer.byteOffset, buffer.byteLength);
let value = dv.getFloat64(0, true);

let serializedDouble = BSON.serialize({ d: value });
expect(serializedDouble.subarray(7, 15)).to.deep.equal(buffer);
let { d: newVal } = BSON.deserialize(serializedDouble, { promoteValues: true });
expect(Number.isNaN(newVal)).to.equal(true);
dariakp marked this conversation as resolved.
Show resolved Hide resolved
});

it('0', () => {
let value = 0;
let orig = new Double(value);
let newVal = twiceSerialized(orig);
expect(value).to.equal(newVal);
});

it('-0', () => {
let value = -0;
let orig = new Double(value);
let newVal = twiceSerialized(orig);
expect(Object.is(newVal, -0)).to.be.true;
});

// TODO (NODE-4335): -0 should be serialized as double
it.skip('-0 serializes as Double', () => {
let value = -0;
let serializedDouble = BSON.serialize({ d: value });
let type = serializedDouble[5];
expect(type).to.not.equal(BSON.BSON_DATA_NUMBER);
dariakp marked this conversation as resolved.
Show resolved Hide resolved
});

it('Number.EPSILON', () => {
let value = Number.EPSILON;
let newVal = twiceSerialized(value);
expect(value).to.equal(newVal);
});
});
});
5 changes: 5 additions & 0 deletions test/node/tools/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -158,6 +158,11 @@ exports.isNode6 = function () {
return process.version.split('.')[0] === 'v6';
};

exports.getNodeMajor = function () {
nbbeeken marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line no-undef
return Number(process.versions.node.split('.')[0]);
};

const getSymbolFrom = function (target, symbolName, assertExists) {
if (assertExists == null) assertExists = true;

Expand Down