Port time measuring and counter utilities

[Alpar Juttner]

These files are affected:

• lemon/time_measure.h
• lemon/counter.h

• lemon/bits/mingw32_time.cc
• lemon/bits/mingw32_time.h

• test/time_measure_test.cc
• test/counter_test.cc

[Alpar Juttner]

I've attached a port of these tools.

It is still questionable what to do with the MinGW compatibility codes which are originally due to Dann Corbit.

[Peter Kovacs]

My comments about the ported files:

• The \author commands should be removed, see 39.

• counter.h:
  • Do we really need NoCounter and SubNoCounter?
  • Should SubNoCounter classes really modify the values of their parents? I don't think so.
  • Wouldn't it be better if SubCounter classes modiefied the value of their parents immediately (not on destruction).
  • I would like counters without title and without printing report on destruction, too (e.g. Counter and CountReport like Timer and TimeReport).

• time_measure.h:
  • The doc should be checked. It should be extended and there are small mistakes in it, too.

• Test files should also be revised.

[Alpar Juttner]

Replying to kpeter:

• counter.h:
  • Do we really need NoCounter and SubNoCounter?

I think, they are really useful as they makes it possible to turn off a counter without actually removing the definition and the increment operators.

Note that NoCounters should not generate any code therefore they do not slow down the algorithms.

• Should SubNoCounter classes really modify the values of their parents? I don't think so.

Well, I could argue for both. The word No may mean here do not count as well as do not report.

• Wouldn't it be better if SubCounter classes modiefied the value of their parents immediately (not on destruction).

I see no advantage of it. On the other hand the code would be less straightforward.

• I would like counters without title and without printing report on destruction, too (e.g. Counter and CountReport like Timer and TimeReport).

What about just using

  int counter;

in this case? :-)

• time_measure.h:
  • The doc should be checked. It should be extended and there are small mistakes in it, too.

More exactly?

• Test files should also be revised.

I agree.

[Alpar Juttner]

A revised port (without mingw support)

[Alpar Juttner]

I uploaded a revised port patch​ without the mingw support, as the current mingw support code in the SVN is not clean enough and I'm lack of the ability to make it better. Instead, another ticket has been created about this issue.

[Peter Kovacs]

Replying to alpar:

I think, they are really useful as they makes it possible to turn off a counter without actually removing the definition and the increment operators.

That's okay. But now SubNoCounter does not meet this concept.

Note that NoCounters should not generate any code therefore they do not slow down the algorithms.

This is true for NoCounter, but false for SubNoCounter. I think SubNoCounter implementation is bad: NoCounters must not count at all!

Well, I could argue for both. The word No may mean here do not count as well as do not report.

I suggest (Sub)Counter and (Sub)NoCounter with a std::ostream* pointer, which has NULL default value in the constructor(s). On destruction this pointer would be checked and the report would be printed only if the pointer is set.

• Wouldn't it be better if SubCounter classes modiefied the value of their parents immediately (not on destruction).

I see no advantage of it. On the other hand the code would be less straightforward.

I see an important advantage: the proper value could be read from the counter at any time.

• I would like counters without title and without printing report on destruction, too (e.g. Counter and CountReport like Timer and TimeReport).

What about just using

  int counter;

in this case? :-)

This is a really straightforward soultion. :-) But only if I don't want to use SubCounters.

However the above suggestions also solve this cases, because counters could be used without an ostream object.

• The doc should be checked. It should be extended and there are small mistakes in it, too.

More exactly?

E.g. in line 351:

  ///This function stops immediately the time counters, i.e. <tt>t.stop()</tt> 
  ///is a faster
  ...

There should be t.halt() instead of t.stop().

[Alpar Juttner]

Replying to kpeter:

I think, they are really useful as they makes it possible to turn off a counter without actually removing the definition and the increment operators.

That's okay. But now SubNoCounter does not meet this concept.

• Sub means a counter the value of which should also counted in its parent counter.
• No means do not report. If the counter incrementation will never be reported, then no increment operation will be placed in the executable.

This seems quite consistent to me.

Note that NoCounters should not generate any code therefore they do not slow down the algorithms.

This is true for NoCounter, but false for SubNoCounter. I think SubNoCounter implementation is bad: NoCounters must not count at all!

See above.

I see an important advantage: the proper value could be read from the counter at any time.

The design goal was to provide a simple tool for counting some kind of events and report the total amount at the end. Not something like a 'progress bar'.

Thus the typical use case is this. You want to count some operations. Then you will use a Counter. Later you want to have finer report on the number of operations, so you setup some SubCounters for the different phases of the algorithm or for different type of operations. When you don't need a certain report anymore, you change it to SubNoCounter. If you need no report at all, you change the main counter to NoCounter.

[Peter Kovacs]

Replying to alpar:

• Sub means a counter the value of which should also counted in its parent counter.
• No means do not report. If the counter incrementation will never be reported, then no increment operation will be placed in the executable.

I see. However the concept I suggested solves all cases: Count/NoCount and Report/NoReport.

There two major problems with your interpretation:

• The class names are really misleading. For me NoCounter means do not count, not do not report. What about using other names? E.g. CountReport and CountNoReport
• "If the counter incrementation will never be reported, then no increment operation will be placed in the executable." It is not true! SubNoCounter increases the value of its parent, so the corresponding code is generated.

My concept would make it possible to switch easily between Count/NoCount and Report/NoReport.

Now counters do not provide soultions for the case, when I have a counter with several subcounters, and I would like to switch on/off some of these counters (the counting not the reporting!).

This seems quite consistent to me.

Except for the fact that the names are misleading.

Note that NoCounters should not generate any code therefore they do not slow down the algorithms.

As I mentioned it severeal times: it is not true for SubNoCounters!

I see an important advantage: the proper value could be read from the counter at any time.

The design goal was to provide a simple tool for counting some kind of events and report the total amount at the end. Not something like a 'progress bar'.

I said any time, not every time. Sometimes it could be useful.

Thus the typical use case is this. You want to count some operations. Then you will use a Counter. Later you want to have finer report on the number of operations, so you setup some SubCounters for the different phases of the algorithm or for different type of operations. When you don't need a certain report anymore, you change it to SubNoCounter. If you need no report at all, you change the main counter to NoCounter.

But what can I do if I would like to switch the counting on/off?

[Peter Kovacs]

What about the reasons and questions I have written?

If the concept will be unchanged, what about using other names? E.g. CountReport or CounterReport. Or using NoSubCounter instead of SubNoCounter.

[Alpar Juttner]

Replying to kpeter:

I suggest (Sub)Counter and (Sub)NoCounter with a std::ostream* pointer, which has NULL default value in the constructor(s). On destruction this pointer would be checked and the report would be printed only if the pointer is set.

I don't like this idea, because

• It would make in impossible to fully turn out the counting automatically when the value is never reported.
• Then a counter reporting to cerr would look like Conter("blabla: ",*std::cerr), which is ugly for me.

[Alpar Juttner]

Replying to kpeter:

There two major problems with your interpretation:

• The class names are really misleading. For me NoCounter means do not count, not do not report. What about using other names? E.g. CountReport and CountNoReport

I don't think it is misleading. You will not be able to use this tool without reading the doc. Once you've read it, the meaning of No should be clear. When you use it, it causes no problem.

• "If the counter incrementation will never be reported, then no increment operation will be placed in the executable." It is not true! SubNoCounter increases the value of its parent, so the corresponding code is generated.

You are wrong here, it is true. It increments the parent's value only if the parent's value is reported. In other words, never means neither by the SubCounter itself nor by its parents.

Note that NoCounters should not generate any code therefore they do not slow down the algorithms.

As I mentioned it severeal times: it is not true for SubNoCounters!

As I tried to explain it several times, it is true even for them!

But what can I do if I would like to switch the counting on/off?

What about removing the ++ operator in this case?

[Alpar Juttner]

Replying to kpeter:

If the concept will be unchanged, what about using other names? E.g. CountReport or CounterReport.

I don't like this idea as I don't feel it would make anything significantly better.

Or using NoSubCounter instead of SubNoCounter.

However, I support this change.

[Peter Kovacs]

Okay, I accept most of your reasons. The concept of counters needn't be changed.

Replying to alpar:

I don't think it is misleading. You will not be able to use this tool without reading the doc. Once you've read it, the meaning of No should be clear. When you use it, it causes no problem.

That was the problem! There aren't any documentation for subcounters (only \todo commands). Writing a clear documentation for these classes would have required much less effort than this whole thread. :)

When I checked the patch file, I read the doc. For NoCounter it says: 'do nothing' version of Counter, and SubNoCounter don't have doc, so it was clear for me that it is the 'do nothing' version of SubCounter. But it isn't.

• "If the counter incrementation will never be reported, then no increment operation will be placed in the executable."

Yes, I see. You are absolutely right here.

Note that NoCounters should not generate any code therefore they do not slow down the algorithms.

As I mentioned it severeal times: it is not true for SubNoCounters!

As I tried to explain it several times, it is true even for them!

No, it is not always true. SubNoCounters don't generate codes only if they are used in a NoCounter. In a Counter they do generate code, since they are not 'do nothing' versions, they have to increase the value of their parents. However it is the only one correct solution for the current concept (SubNoCounter counts but does not report). That's why I wanted to change the concept or the names. Maybe it was misleadng even for you as you wrote NoCounters should not generate any code. It would be true according to my suggestions, but it isn't always true now. Am I right?

[Alpar Juttner]

No, it is not always true. SubNoCounters don't generate codes only if they are used in a NoCounter. In a Counter they do generate code, since they are not 'do nothing' versions, they have to increase the value of their parents. However it is the only one correct solution for the current concept (SubNoCounter counts but does not report). That's why I wanted to change the concept or the names. Maybe it was misleadng even for you as you wrote NoCounters should not generate any code. It would be true according to my suggestions, but it isn't always true now. Am I right?

This whole thing is a matter of right documentation not the matter of concept. Am I right?

[Alpar Juttner]

What I suggest is to do the SubNo -> NoSub change, than merge them back to the main branch. Of course, the documentation still must be improved, thus this ticket should be left open, or another one should be created for the doc improvements.

Are you agree with it?

[Peter Kovacs]

Replying to alpar:

This whole thing is a matter of right documentation not the matter of concept. Am I right?

Yes, you are right. (Writing a clear documentation for these classes would have required much less effort than this whole thread. :))

Replying to alpar:

What I suggest is to do the SubNo -> NoSub change, than merge them back to the main branch.

I agree.

Of course, the documentation still must be improved, thus this ticket should be left open, or another one should be created for the doc improvements.

I suggest opening a new ticket about the doc and closing this one.

[Alpar Juttner]

The port went into the main branch, see [82a2639a05bb], [137278093143] and [91c0fed3181e].

A new ticket is created for the doc improvement, see ticket:82