benchmark: refactor to remove duplicate code

[ian-perkins]

The dns benchmark script contained an if-then-else branch which
executed almost the same code in both cases, apart from the number
of args passed to the core dns lookup function.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• documentation is changed or added
• commit message follows commit guidelines

Affected core subsystem(s)

[BridgeAR]

Thanks a lot for your contribution but the benchmark was written as it is out of a reason to make sure as little other code as possible will interfere with the part tested as possible. Your addition adds a overhead for both types and it is not completely clear why one or the other is faster or not. Furthermore reducing n to such a low number prevents the function from being fully limited by the CPU and the results become less significant.

Because of these reasons I am -1 on landing this.

[ian-perkins]

TBH my motivation for doing this was seeing two blocks of code which vary only in the number of args passed to dns.lookup so using the spread operator seems to me to make sense and cleans up the code (IMO). Are you saying that the spread operator has a performance impact?

I can change n back to 5e6 although the test harness for dns (which I created as part of this PR) actually overrides this by setting n=1

[jasnell]

Yes, the spread operator has a non-trivial performance impact here. What I would recommend is running a comparison of the benchmark before and after the change to gauge how much. If the difference is not significant, it may be ok

[BridgeAR]

@ian-perkins I am not sure what your comment about the test has to do with the benchmark itself and as @jasnell pointed out - the spread operator definitely has a performance impact. Expect most things to have a impact in general, especially in microbenchmarks.
In most cases it is totally fine and somewhat expected if a benchmark has a bit of duplicated code.

[ian-perkins]

Yes, I now see what you mean. I ran some tests and the refactored code is almost twice as slow! Lesson learned - I'll pay more attention in future...

Do I need to do anything extra to get the PR rejected?

Thanks for your patience

[BridgeAR]

@ian-perkins you can always click on the close button right next to "Comment" if you you think it makes sense to close a PR (I hope I understood your comment correct).

Thanks a lot for your contribution anyway!