Reduce clock syscalls #4303
Reduce clock syscalls #4303
Conversation
Gate the sample of the clock by the tessedit_timing_debug flag, which is the only time it gets used anyway. This eliminates unnecessary clock_gettime() system calls.
src/ccmain/control.cpp
Outdated
| clock_t start_t; | ||
| if (tessedit_timing_debug) { | ||
| start_t = clock(); | ||
| } |
There was a problem hiding this comment.
Are you sure that the if statement is less expensive than the clock() call?
There was a problem hiding this comment.
The call to clock() results in a vdso syscall to the operating system. If you are running tesseract in batch mode with multiple copies running on the same machine, this seemingly innocuous call could result in a storm of useless syscalls to the OS. It is definitely more expensive than a single conditional that all branch predictors would get correct.
There was a problem hiding this comment.
Regardless of whether one believes this is slower or faster, the start_t variable has no consumers unless tessedit_timing_debug is enabled.
| if (tessedit_timing_debug) { | ||
| clock_t ocr_t = clock(); |
There was a problem hiding this comment.
This part of the commit is a good change.
|
@stweil: what about switching to c++11 |
| if (tessedit_timing_debug) { | ||
| clock_t ocr_t = clock(); | ||
| tprintf("%s (ocr took %.2f sec)\n", word_data->word->best_choice->unichar_string().c_str(), | ||
| static_cast<double>(ocr_t - start_t) / CLOCKS_PER_SEC); |
There was a problem hiding this comment.
The bad news is that g++ is not clever enough and produces a warning with this PR:
src/ccmain/control.cpp:1373:39: warning: 'start_t' may be used uninitialized [-Wmaybe-uninitialized]
There was a problem hiding this comment.
Sorry about that. I will initialize it.
There was a problem hiding this comment.
But init is exactly wasted cycles again?
There was a problem hiding this comment.
You need to use [[maybe_unused]] attribute here.
There was a problem hiding this comment.
What g++ version?
g++ (Debian 12.2.0-14) 12.2.0
There was a problem hiding this comment.
Interesting to check it on gcc-14
There was a problem hiding this comment.
Apple clang version 15.0.0 (clang-1500.3.9.4) and g++-14 (Homebrew GCC 14.1.0_2) 14.1.0 show the same warning. This is not surprising because the compiler must assume that tessedit_timing_debug might be changed between the two conditional statements. Therefore a local assignment const bool timing_debug = tessedit_timing_debug; helps. It fixes the warning for g++-14, but not for clang 15.
|
I wonder whether we should simply remove this timing code (or compile it conditionally). The debug messages are printed per word with a resolution of 10 ms. In a short test I got the results 0.00 sec, 0.01 sec and 0.02 sec. That's not really helpful. |
|
Apply for now and postpone to timing/debugging related refactoring. |
|
I have run a short test to see the effect of less calls of |
|
The effects will be more egregious when running in batch mode, e.g. with 192 instances executing simultaneously on a modern many-core server. |
I noticed significant calls to
clock_gettimeso I investigated and found that we could eliminate them when they are not in use. This was found from the continuing scrutiny of SPEC CPUv8 benchmark development. @zdenopGate the sampling of the clock by the
tessedit_timing_debugflag, which is the only time it gets used anyway. This eliminates unnecessary clock_gettime() system calls.