Use performance.now() instead of Date.now()...

[bitifet]

Replaced Date.now() for performance.now() (wherever applicable) to improve timing accuracy.

Date.now() is prone to system clock shifts, while performance.now() offers a high-resolution, monotonic clock for consistent, reliable measurements.

Not sure, but it could be involved in timeout issues we experience in VM environment but, anyway, this is the best way to measure time.

Added faliback code to support node prior to 16.0.0 (where perf_hook were not globally exposed by default) and even prior to 8.5.0 (defaulting to Date.now()).

Verified tests passes with node 22.12.0.

Attempted to pass with node prior to 16.0.0 but faced issues due to picked dependencies versions and other code of node_postgres that I guess it haven't been tested with so old node versions.

(I attempted to support at least from 8.0.0 which is declared in engines.node of packages/pg/package.json)

References:

https://nodejs.org/docs/latest-v8.x/api/perf_hooks.html#perf_hooks_performance_now

https://w3c.github.io/hr-time/

[charmander]

Not sure, but it could be involved in timeout issues we experience in VM environment

This pull request currently only changes benchmarking code that isn’t used by library consumers, so it definitely won’t fix those issues.

[bitifet]

Not sure, but it could be involved in timeout issues we experience in VM environment

This pull request currently only changes benchmarking code that isn’t used by library consumers, so it definitely won’t fix those issues.

You're absolutely right.

I shouldn't have even mentioned that. It's just that was the reason why I got checking that.

I only grepped and checked that the changes wouldn't break expected behaviour.

As you said, it doesn't fixes anything, at least from the consumers point of view. But, anyway, I still think that it is better than before: At the end, those benchmarks are there for a reason and the more accurate they could be the better.

Anyway, if you feel it's better to reject the PR, don't worry and go ahead.

Regards.

[brianc]

Thanks for submitting this. One small change request...we don't support node < 16 anymore so its okay to not have the backwards compatibility for older, unsupported versions of node. Would probably simplify by removing that code.

[bitifet]

Thanks for submitting this. One small change request...we don't support node < 16 anymore so its okay to not have the backwards compatibility for older, unsupported versions of node. Would probably simplify by removing that code.

Job done.

Just little question: If node versions prior to 16 aren't supported, shouldn't engines.node in pg/package.json be ">= 16.0.0" instead of ">= 8.0.0"?

I guess the rest of packages' package.json files lack the engines section to avoid having to maintain each of them since all are dependencies of the pg package.

I would have changed it directly, but not being sure if I might be overlooking something I have preferred to be conservative.

[brianc]

Thanks @bitifet!

Just little question: If node versions prior to 16 aren't supported, shouldn't engines.node in pg/package.json be ">= 16.0.0" instead of ">= 8.0.0"?

yes, you're right though that's kinda breaking change of sorts? If someone wants to go yolo and install this on an older version of node...hmmmm they can try. I feel like if you're on a version of node older than end of life support I dunno what to tell you...but always pin your dependency versions!

[bitifet]

My pleasure.

yes, you're right though that's kinda breaking change of sorts? If someone wants to go yolo and install this on an older version of node...hmmmm they can try. I feel like if you're on a version of node older than end of life support I dunno what to tell you...but always pin your dependency versions!

I see, but this does not prevent him to install the package in an older version.

Unless engine-strict is set, engines.node is advisory, not mandatory, and even in that case it can be overridden with --force.

[brianc]

I see, but this does not prevent him to install the package in an older version.

I didn't know any of that! I'll do a PR to change the engines soon. :)