update test-performance use setImmediate instead of setTimeout
#22093
Conversation
fesebuv
changed the title
test-performance use setInmmediate instead of setTimeouttest-performance use setImmediate instead of setTimeout
There was a problem hiding this comment.
I'll leave it to the original authors to decide the validity of this change but this PR should be restricted to the described change.
E.g., the change to use forEach doesn't actually improve the code. And the style change of adding brackets around an if is also not meaningful.
| }; | ||
|
|
||
| function checkNodeTiming(props) { | ||
| Object.keys(props).forEach((prop) => { |
There was a problem hiding this comment.
Why this change? This isn't faster or better.
There was a problem hiding this comment.
No is not, but if you are invoking a chain-able method for an array, might as well chain and iterate no?
| @@ -4,8 +4,9 @@ const common = require('../common'); | |||
| const assert = require('assert'); | |||
| const { performance } = require('perf_hooks'); | |||
|
|
|||
| if (!common.isMainThread) | |||
| if (!common.isMainThread) { | |||
There was a problem hiding this comment.
I'm just trying to improve the code a little bit sir.
test/sequential/test-performance.js
Outdated
| startTime: 0, | ||
| checkNodeTiming(timingParams); | ||
|
|
||
| setImmediate(() => { |
There was a problem hiding this comment.
I would assume there's a reason the original test case used both setImmediate and setTimeout.
…Timeout & refactor do not need to redifine duration
fesebuv
force-pushed
the
fesebuv-update-test
branch
from
ad115db to
fa3f2a0
Compare
The |
|
Based on the following comment #20128 (comment) I will follow up by closing this pr. Thanks for the review @Trott @apapirovski |
The following
prreduces the time assertions take by replacingsetTimeoutbysetImmediatethat should make test run faster.Also included some code refactor, make improve code readability.
Run
Should see time reducing from real
~0m2.106storeal ~0m0.111sChecklist
make -j4 test(UNIX), orvcbuild test(Windows) passes