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

buffer: optimize readDouble and readFloat methods #17775

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
46 changes: 46 additions & 0 deletions benchmark/buffers/buffer-read-float.js
@@ -0,0 +1,46 @@
'use strict';
const common = require('../common.js');

const bench = common.createBenchmark(main, {
noAssert: ['false', 'true'],
type: ['Double', 'Float'],
endian: ['BE', 'LE'],
value: ['zero', 'big', 'small', 'inf', 'nan'],
millions: [1]
});

function main(conf) {
const noAssert = conf.noAssert === 'true';
const len = +conf.millions * 1e6;
const buff = Buffer.alloc(8);
const type = conf.type || 'Double';
const endian = conf.endian;
const fn = `read${type}${endian}`;
const values = {
Double: {
zero: 0,
big: 2 ** 1023,
small: 2 ** -1074,
inf: Infinity,
nan: NaN,
},
Float: {
zero: 0,
big: 2 ** 127,
small: 2 ** -149,
inf: Infinity,
nan: NaN,
},
};
const value = values[type][conf.value];

buff[`write${type}${endian}`](value, 0, noAssert);
const testFunction = new Function('buff', `
for (var i = 0; i !== ${len}; i++) {
buff.${fn}(0, ${JSON.stringify(noAssert)});
}
`);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do not really see the necessity for new Function here. It would be nice to just set a var to the specific function as in

for (var i = 0; i !== len; i++) {
  fn(0, noAssert);
}

(with a few other changes necessary for this)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't work. buf[fn](0, noAssert) could work but takes a different, non-default code path in V8 (keyed vs. named load.)

That said, I didn't invent this, the other buffer benchmarks do the same thing.

bench.start();
testFunction(buff);
bench.end(len / 1e6);
}
89 changes: 65 additions & 24 deletions lib/buffer.js
Expand Up @@ -31,10 +31,6 @@ const {
indexOfBuffer,
indexOfNumber,
indexOfString,
readDoubleBE: _readDoubleBE,
readDoubleLE: _readDoubleLE,
readFloatBE: _readFloatBE,
readFloatLE: _readFloatLE,
swap16: _swap16,
swap32: _swap32,
swap64: _swap64,
Expand Down Expand Up @@ -1201,35 +1197,80 @@ Buffer.prototype.readInt32BE = function readInt32BE(offset, noAssert) {
};


Buffer.prototype.readFloatLE = function readFloatLE(offset, noAssert) {
offset = offset >>> 0;
if (!noAssert)
checkOffset(offset, 4, this.length);
return _readFloatLE(this, offset);
// For the casual reader who has not at the current time memorized the
// IEEE-754 standard in full detail: floating point numbers consist of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yes, IEEE-754... Lite-afternoon reading that goes perfect with coffee and scones.

// a fraction, an exponent and a sign bit: 23+8+1 bits for single precision
// numbers and 52+11+1 bits for double precision numbers.
//
// A zero exponent is either a positive or negative zero, if the fraction
// is zero, or a denormalized number when it is non-zero. Multiplying the
// fraction by the smallest possible denormal yields the denormalized number.
//
// An all-bits-one exponent is either a positive or negative infinity, if
// the fraction is zero, or NaN when it is non-zero. The standard allows
// both quiet and signalling NaNs but since NaN is a canonical value in
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extreme nit s/signalling/signaling if we're favouring the use of the American spellings ;-)

// JavaScript, we cannot (and do not) distinguish between the two.
//
// Other exponents are regular numbers and are computed by subtracting the bias
// from the exponent (127 for single precision, 1023 for double precision),
// yielding an exponent in the ranges -126-127 and -1022-1024 respectively.
//
// Of interest is that the fraction of a normal number has an extra bit of
// precision that is not stored but is reconstructed by adding one after
// multiplying the fraction with the result of 2**-bits_in_fraction.


function toDouble(x0, x1) {
const frac = x0 + 0x100000000 * (x1 & 0xFFFFF);
const expt = (x1 >>> 20) & 2047;
const sign = (x1 >>> 31) ? -1 : 1;
if (expt === 0) {
if (frac === 0) return sign * 0;
return sign * frac * 2 ** -1074;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be nice to use brackets here for better readability. The same applies to all places where the exponential operator is used.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I kind of disagree since it's unambiguous: exponentiation has higher precedence than multiplication. Binary expressions with superfluous parens grinds my gears because it suggests its author didn't know the precedence rules.

I would have written it as sign * frac * 2**-1074 if eslint didn't complain about it, but it does.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm good with this without brackets as well.

} else if (expt === 2047) {
if (frac === 0) return sign * Infinity;
return NaN;
}
return sign * 2 ** (expt - 1023) * (1 + frac * 2 ** -52);
}


function toFloat(x) {
const frac = x & 0x7FFFFF;
const expt = (x >>> 23) & 255;
const sign = (x >>> 31) ? -1 : 1;
if (expt === 0) {
if (frac === 0) return sign * 0;
return sign * frac * 2 ** -149;
} else if (expt === 255) {
if (frac === 0) return sign * Infinity;
return NaN;
}
return sign * 2 ** (expt - 127) * (1 + frac * 2 ** -23);
}


Buffer.prototype.readDoubleBE = function(offset, noAssert) {
const x1 = this.readUInt32BE(offset + 0, noAssert);
const x0 = this.readUInt32BE(offset + 4, noAssert);
return toDouble(x0, x1);
};


Buffer.prototype.readFloatBE = function readFloatBE(offset, noAssert) {
offset = offset >>> 0;
if (!noAssert)
checkOffset(offset, 4, this.length);
return _readFloatBE(this, offset);
Buffer.prototype.readDoubleLE = function(offset, noAssert) {
const x0 = this.readUInt32LE(offset + 0, noAssert);
const x1 = this.readUInt32LE(offset + 4, noAssert);
return toDouble(x0, x1);
};


Buffer.prototype.readDoubleLE = function readDoubleLE(offset, noAssert) {
offset = offset >>> 0;
if (!noAssert)
checkOffset(offset, 8, this.length);
return _readDoubleLE(this, offset);
Buffer.prototype.readFloatBE = function(offset, noAssert) {
return toFloat(this.readUInt32BE(offset, noAssert));
};


Buffer.prototype.readDoubleBE = function readDoubleBE(offset, noAssert) {
offset = offset >>> 0;
if (!noAssert)
checkOffset(offset, 8, this.length);
return _readDoubleBE(this, offset);
Buffer.prototype.readFloatLE = function(offset, noAssert) {
return toFloat(this.readUInt32LE(offset, noAssert));
};


Expand Down
48 changes: 0 additions & 48 deletions src/node_buffer.cc
Expand Up @@ -719,49 +719,6 @@ static inline void Swizzle(char* start, unsigned int len) {
}


template <typename T, enum Endianness endianness>
void ReadFloatGeneric(const FunctionCallbackInfo<Value>& args) {
THROW_AND_RETURN_UNLESS_BUFFER(Environment::GetCurrent(args), args[0]);
SPREAD_BUFFER_ARG(args[0], ts_obj);

uint32_t offset = args[1]->Uint32Value();
CHECK_LE(offset + sizeof(T), ts_obj_length);

union NoAlias {
T val;
char bytes[sizeof(T)];
};

union NoAlias na;
const char* ptr = static_cast<const char*>(ts_obj_data) + offset;
memcpy(na.bytes, ptr, sizeof(na.bytes));
if (endianness != GetEndianness())
Swizzle(na.bytes, sizeof(na.bytes));

args.GetReturnValue().Set(na.val);
}


void ReadFloatLE(const FunctionCallbackInfo<Value>& args) {
ReadFloatGeneric<float, kLittleEndian>(args);
}


void ReadFloatBE(const FunctionCallbackInfo<Value>& args) {
ReadFloatGeneric<float, kBigEndian>(args);
}


void ReadDoubleLE(const FunctionCallbackInfo<Value>& args) {
ReadFloatGeneric<double, kLittleEndian>(args);
}


void ReadDoubleBE(const FunctionCallbackInfo<Value>& args) {
ReadFloatGeneric<double, kBigEndian>(args);
}


template <typename T, enum Endianness endianness>
void WriteFloatGeneric(const FunctionCallbackInfo<Value>& args) {
Environment* env = Environment::GetCurrent(args);
Expand Down Expand Up @@ -1276,11 +1233,6 @@ void Initialize(Local<Object> target,
env->SetMethod(target, "indexOfNumber", IndexOfNumber);
env->SetMethod(target, "indexOfString", IndexOfString);

env->SetMethod(target, "readDoubleBE", ReadDoubleBE);
env->SetMethod(target, "readDoubleLE", ReadDoubleLE);
env->SetMethod(target, "readFloatBE", ReadFloatBE);
env->SetMethod(target, "readFloatLE", ReadFloatLE);

env->SetMethod(target, "writeDoubleBE", WriteDoubleBE);
env->SetMethod(target, "writeDoubleLE", WriteDoubleLE);
env->SetMethod(target, "writeFloatBE", WriteFloatBE);
Expand Down