update test-performance use setImmediate instead of setTimeout#22093
update test-performance use setImmediate instead of setTimeout#22093fesebuv wants to merge 1 commit intonodejs:masterfrom
test-performance use setImmediate instead of setTimeout#22093Conversation
test-performance use setInmmediate instead of setTimeouttest-performance use setImmediate instead of setTimeout
apapirovski
left a comment
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.
test/sequential/test-performance.js
Outdated
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?
test/sequential/test-performance.js
Outdated
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
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
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