brianc / node-pg-types

Type parsing for node-postgres
267 stars 54 forks source link

perf: prefer parse int with + instead parseInt() #142

Closed H4ad closed 1 year ago

H4ad commented 1 year ago

Closes #141

Based on nodejs-benchmarks about parseInt and + operator, using + is way faster than parseInt for large numbers.

Here's the benchmark with the new parser:

Using parseInt(x, 10) - small number (2 len) x 300,130,498 ops/sec ±1.74% (92 runs sampled)
Using parseInt(x, 10) - big number (10 len) x 16,455,360 ops/sec ±1.38% (95 runs sampled)
Using + - small number (2 len) x 1,130,810,993 ops/sec ±0.59% (94 runs sampled)
Using + - big number (10 len) x 1,141,065,805 ops/sec ±0.08% (97 runs sampled)
Benchmark ```js const Benchmark = require('benchmark') const suite = new Benchmark.Suite; function parseIntegerV1 (string) { return parseInt(string, 10); } function parseIntegerV2 (string) { if (string === '' || string === undefined || string === null) { return NaN } return +string } suite .add('Using parseInt(x, 10) - small number (2 len)', function () { parseIntegerV1('5', 10) }) .add('Using parseInt(x, 10) - big number (10 len)', function () { parseIntegerV1('9999999999', 10) }) .add('Using + - small number (2 len)', function () { parseIntegerV2('5') }) .add('Using + - big number (10 len)', function () { parseIntegerV2('9999999999') }) .on('cycle', function(event) { console.log(String(event.target)) }) .run({ 'async': false }); ```
charmander commented 1 year ago

The input can’t be any of those values, so how about replacing parseInteger with Number?

H4ad commented 1 year ago

@charmander I think that could be possible but for the sake of compatibility with the old method, I think the performance will not be hurt to keep as it is with that if guard against null.

charmander commented 1 year ago

Why do we care about compatibility with these specific out-of-scope cases and not the rest of them? See also #122.

H4ad commented 1 year ago

@charmander In this case, make sense to me to just use Number instead, @bendrucker what do you think?

bendrucker commented 1 year ago

Yeah, agree that we should be continuing to roll back these cases where every parser handles null (the same way) in favor of the caller handling it. So Number would be fine.