-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
perf: pre allocate array instead of push item #3250
base: master
Are you sure you want to change the base?
Conversation
Thanks for bringing this up. Do you have any benchmark results? |
The PR is 20% faster. Tested only on node 20. |
20% faster on what benchmark? |
I have made a benchmark of single method, but moking the db/query logic, just for test if Preallocating array has a performance benefit, eg consumeFields |
pre allocate array is generally faster compared to other methods, push is the slower |
@charmander after fixed dev container with #3251 I have also run the pg-native bench https://github.com/brianc/node-postgres/blob/master/packages/pg-native/bench/index.js TL;TR The benchmark shows no improvement. But looking at the benchmark code, this doesn't appear to be very accurate PR
MASTER
|
Hi @charmander also on the vanilla "result object" the array length is pre allocate, why not on native? https://github.com/brianc/node-postgres/blob/master/packages/pg/lib/result.js#L51 This PR just align the behaviour. Side note: Side note: preallocate and fill is better, eg.: const size = 1_000;
const arr = new Array(size).fill(null);
for(let i = 0; i<size; i++){
arr[i] = {foo: "foo"};
} see benchmark |
just a little performance improvement. Preallocating array instead of push item