Added benchmark framework and improvements to toTextRange

[andralex]

This is a pretty cool benchmark framework that makes it easy to collect good-quality data about function run times. As a proof of concept I integrated the framework with Phobos itself. Have at it!

[donc]

Very useful, but where should this stuff go?
My feeling is that benchmarking stuff doesn't have any business being in std.datetime. In fact, I'd even say that it's important that it's not in there, since std.datetime pulls in so much code, which can interfere with timings.
I would expect to see a module with a name like std.benchmark (or a replacement for std.perf) which would ultimately report things like memory usages, as well as time. One of the most basic features for such a module would be to restrict execution to a particular core (Using SetProcessAffinity() on Windows, I forget what the *nix equivalent is).

[mrmonday]

I think this should be in std.benchmark too, with some other useful benchmark related functions. http://blog.regehr.org/archives/330 has a nice list of things that need doing to minimize noise. Perhaps columns for total time, or the time as a percentage per module would also be useful - it could be adapted to accept multiple modules then give a percentage time overall too.
I also think it would be a good idea to mention in the comments that it only benchmarks things you have benchmark functions for - it's still a good idea to run dmd -profile and find out what's using up time in your application.

[mrmonday]

Another idea, perhaps there should be an option to output results in a machine readable format so charts etc can be generated - CSV is probably the simplest to support here.

[DmitryOlshansky]

Nitpicks:

31043: Both examples won't work:
with std.file.write is two parameter function, it should have some filename. Maybe switch it std.stdio.writeln
in second one with appender put(a, 'x'); --> put(s, 'x');

31105: Should we also place a check - if benchmark_xxx is callable?

And name - benchmark2 is odd, I somehow expected that it works only with 2 functions or some such. If it's user accessible artifact we need a better name.

Overall seems cool, and using a nop loop as a baseline is a great idea.

[andralex]

I agree that benchmark doesn't belong in std.datetime. At least in Google's libs, benchmarking comes packaged with unittesting, but we don't have a unittest package :o). If deemed appropriate, we could add a new module std.benchmark.

[andralex]

BTW, to benchmark Phobos on Linux you'd say:

make BENCHMARK=1 BUILD=release unittest

To benchmark one module only:

make BENCHMARK=1 BUILD=release generated/linux/release/32/unittest/std/some_module

Currently there are only three benchmarks, but the way I see things going forward is that all of Phobos' important entities will have benchmarks that will be improved on a need basis and scrutinized with each release.

[andralex]

@mrmonday: I have access to total time, but that is confusing and irrelevant given that it's influenced by the total number of iterations. The columns contain numbers that can be directly compared across functions.

[jmdavis]

I definitely support the idea of moving the benchmarking code out of std.datetime. It's related but still pretty separate, and std.datetime is large enough that if there's an obvious case where something might be better off in a separate module, then it should probably be put in a separate module. Creating a std.benchmark for the benchmarking functions makes good sense, and I'd even suggest moving StopWatch there as well. Then all of the timing code will be in one module.

[andralex]

OK, this became reviewable. Have at it -- thanks!

[dnadlinger]

Is that header still needed?

[jmdavis]

I notice that your changes yank out from std.datetime StopWatch and the benchmarking functions which currently exist in std.datetime without any attempt at avoiding code which currenly uses them. It would probably be best to have std.datetime publicly import std.benchmark at least for a few releases. As it is, any code using StopWatch or the existing benchamarking functions will break.

[jmdavis]

Is there a good reason why instead of having a struct or class with the benchmarking functions on it and which then has theStopWatch as a member variable that you made theStopWatch a module variable and made the functions which use it free functions? About the only thing that I can think of is that it makes no sense to have more than one at once, and you didn't want to bother with a singleton class which held it. It does strike me as very weird though. You don't normally want module variables to be mutable.

[andralex]

I will add the import. Regarding benchmarkPause/benchmarkResume, it's just KISS at work. There's only one benchmark running at most, so adding the struct/class scaffolding to give the illusion there may be multiple instances out there, to then do even more work to prevent those multiple instances from being created - there's no advantage to doing so.

[andralex]

Integrated feedback (thanks!)

[andralex]

All -- please let me know if this should instead follow the standard review process. If so, we're looking for a review manager. Thanks!

[ghost]

You left bar undefined, add this here:
static void bar() {}

[ghost]

You also didn't update the windows makefile. I guess you use the Linux makefile via Wine? I've updated it but I'm not sure how I can "glue" my changeset to your pull request. (it's not in my repo yet).

[jmdavis]

Is there any reason to keep this pull request open at this point? It was decided that we should review it in the newsgroup, and you (Andrei) said that you were reworking it. I'd argue that it would be better to just close this one for now, and then you can create a new one when we're actually ready to merge it in.