optimization timers promisify #9485
Conversation
| const fn = defineCustomPromisify(interval, require("node:timers/promises").setInterval); | ||
| return fn.$apply(this, arguments); | ||
| if (!timersPromises) timersPromises = require("node:timers/promises"); | ||
| return timersPromises.setInterval.$apply(this, arguments); | ||
| }); | ||
| } |
There was a problem hiding this comment.
I like this @sirenkovladd! Can you provide a little microbenchmark result. We have gone back and forth on this pattern and a microbenchmark would help solidify a path.
There was a problem hiding this comment.
the following optimization pattern should not affect performance. unlike in user code, require is inlined to the following code:
(@getInternalField(@internalModuleRegistry, 21/*node:events*/) || @createInternalModuleById(21/*node:events*/));
(this transform is done in internal-module-registry-scanner.ts, called in sliceSourceCode)
the first half of this is already a cache lookup, specifically a native array access into the JSInternalFieldObjectImpl in InternalModuleRegistry.h/cpp. this lookup should be incredibly instant and very JIT friendly. reminder that getInternalField is a bytecode intrinsic (see emit_intrinsic_getInternalField). then we check if it is not defined and take the || path which will call into a native function @createInternalModuleById (see jsCreateInternalModuleById). the only thing that is slow is the latter call, which is already guarded by a conditional expression.
my stance on this pattern is that it is a unnecessary optimization, and that we should actually reverse any places that we do this.
however ... i have not benchmarked the above, and i'm open to being proved wrong.
but then again, i dont think there is any application that the initialization of util.promisify on the timer becomes the slow point of an application.
|
I created a microbenchmark and could see no noticeable win from this (microbenchmarks hate added scope/closures) and saw no benefits from hoisting here. We can remove the pattern. @sirenkovladd If you open to helping on this front a PR to remove this kind of pattern (if it exists elsewhere) would be helpful. |
What does this PR do?
How did you verify your code works?
bun-debug test test-file-name.test)