[cleanup] Remove old code in Dune::Timer

Dune::Timer includes two alternative implementations for measuring time. Historically one was based on getrusage() and the other one on std::clock(). The former was the default and the latter could be enabled by defining a macro. Both in fact measured the time spend computing by the process.

In fa43f4bf the default code path was changed from getrusage() to std::chrono::high_resolution_clock::now() while std::clock() could still be enabled by the macro. This was in fact a breaking change because the new default version measures the elapsed real time.

This patch removed the non-default version based on std::clock because:

• The breaking change happened almost 11 years so one can consider the new behavior the established 'correct' version.
• The macro switch was neither documented nor tested.
• Setting the macro manually in user code is error prone.
• Both versions do completely different things leading to different measurements. It's not even clear that one produces larger numbers in general: std::clock adds up time spend computing in all threads of the process. Thus time in concurrent threads is added up, while the time a thread is sleeping is not counted.

There a minor grain of salt: The documentation still documented the old behaviour and instead of 'fixing' the code this patch adjusts the documentation to established reality.

Any objections to this?

No, go ahead.

I faintly recall, that there have been changes about precision around that time. Not sure whether that was this. Would it be possible to put the commit hash into normal text to see a link?

changed the description

Thanks, go ahead with merging!

What about deprecating the whole Dune::Timer? Isn't <chrono> enough and an established standard compared to a Duneism (because Dune pre-dates C++11).

While checking out this code I realized that the timer does not have a proper test. This object is overall used in:

• timing.cc: Not tested (probably because it requires dune-istl).
• mpi_collective_benchmark.cc: Not a test at all.
• selectiontest.cc: Not a proper test of the interface, just a time report of some work.

So how about cleaning timing.cc too (e.g. removing the file)?

added 2 commits

• 7a6b38e2 - Store timer duration in native duration type & Add proper test for timer
• 96925d0a - Merge branch 'feature/store-timer-duration' into 'feature/cleanup-timer'

Compare with previous version

added 12 commits

• 96925d0a...8f1177e4 - 10 commits from branch master
• 28cebf2a - [cleanup] Remove old code in Dune::Timer
• a0ccdbe5 - Store timer duration in native duration type & Add proper test for timer

Compare with previous version

Once the wording in the comment is resolved, I'd like to merge this.

resolved all threads

added 1 commit

• 638f4378 - Fix typo

Compare with previous version

added 7 commits

• 638f4378...15b2c1ee - 4 commits from branch master
• 5b13e205 - [cleanup] Remove old code in Dune::Timer
• fb50e822 - Store timer duration in native duration type & Add proper test for timer
• 373b9675 - Fix typo

Compare with previous version

enabled an automatic merge when all merge checks for 373b9675 pass

merged

mentioned in commit d2c74816