I just created a PR aimed at replacing the few pieces of Phobos that use TickDuration in their API, and it involves creating the std.benchmark module to hold the updated benchmarking functions, since they can't be overloads of the existing ones in std.datetime given that return types are involved. So, the new functions need to go in a different module. Putting them in std.benchmark made perfect sense given that that's where we were looking at putting them before when Andrei's std.benchmark was reviewed (and ultimately dropped). Normally, adding a module to Phobos would involve an extended review in the newsgroup, but in this case, it's just updated versions of existing functions moved to a new module. So, I don't think that a review of that level is required, but I figured that I'd bring it up in the newsgroup just in case.

https://github.com/D-Programming-Language/phobos/pull/3695

- Jonathan M Davis

LOL. I got the title wrong - std.benchmarking instead of std.benchmark. Oh well...

- Jonathan M Davis

Should the examples have `pragma(inline, false)` on the benchmarked functions? I'm not so worried about inlining as I am about const folding the benchmarked expressions away.

Concerning the module review I tend to agree with you. It would only be ... well if I said in std.datetime the functions were not so prominent, and went under the radar, but as their own module they have to add enough value to warrant that. But I am ok with that, too. std.benchmark is not something you'd use in a production environment on a daily basis like you would std.json, std.logger or std.allocator and you didn't add new API.

If there _was_ a review for a new module I would swamp your progress with feature requests like "validation of return value of benchmarked functions" or "benchmark for x seconds with y iterations of the inner loop for comparing benchmarks for cases where one function is orders of magnitudes faster than the other". And I'm sure others have lots of ideas, too and it will turn into an endless bike-shedding discussion and take another 6 months to get this merged and no one wants that, right?

-- 
Marco

On Monday, 5 October 2015 at 15:51:58 UTC, Marco Leise wrote:
> Should the examples have `pragma(inline, false)` on the benchmarked functions? I'm not so worried about inlining as I am about const folding the benchmarked expressions away.

I'm not sure that it's a good idea to start pasting pragmas for inlining all over the place rather than letting the inliner do its job, but regardless, there have been enough complaints about how pragma(inline...) is designed that I don't know if we can even rely on it staying the way it is right now.

> If there _was_ a review for a new module I would swamp your progress with feature requests like "validation of return value of benchmarked functions" or "benchmark for x seconds with y iterations of the inner loop for comparing benchmarks for cases where one function is orders of magnitudes faster than the other". And I'm sure others have lots of ideas, too and it will turn into an endless bike-shedding discussion and take another 6 months to get this merged and no one wants that, right?

Andrei previously had a proposal for std.benchmark which added a lot more, but it died in the review process (primarily due to it relying on static constructors to do some of what it was doing IIRC). We may very well get another proposal from him or someone else in the future. But this PR is entirely about replacing existing benchmarking functions with versions that use Duration and MonoTime instead of TickDuration and not about adding functionality. Major additions like Andrei worked on before can go through the review process as normal, and it's certainly not on my todo list. I'm behind enough on that as it is. As it stands, the main reason that the PR creates std.benchmark is because the new functions cannot got in std.datetime without conflicting with the functions that they're replacing (though I do think that the benchmarking functions are better placed in std.benchmark anyway, and Andrei's proposal did include moving them there).

- Jonathan M Davis