URL: https://github.com/SSSD/sssd/pull/5525 Author: alexey-tikhonov Title: #5525: Log performance improvements. Action: opened
PR body: """ A set of patches intended to improve log performance.
As a result, time spent in a trivial test: ``` for (c = 0; c < 10000000; ++c) { DEBUG(SSSDBG_OP_FAILURE, "Message %d\n", c); } ``` with "--debug-timestamps=1 --debug-microseconds=0" is reduced on my machine from ~34 to ~14 seconds (i.e. ~55% faster). """
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5525/head:pr5525 git checkout pr5525
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
Label: +Waiting for review
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
pbrezina commented: """ Very nice. At this point, you cache the whole string. If it differs, you generate the whole string again. But most often the only thing that changes is only the last "seconds" part. Do you think it would make a difference if we cache only (or as a level 2 cache) the date and time without seconds that changes rarely? """
See the full comment at https://github.com/SSSD/sssd/pull/5525#issuecomment-789647627
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
alexey-tikhonov commented: """
At this point, you cache the whole string. If it differs, you generate the whole string again. But most often the only thing that changes is only the last "seconds" part. Do you think it would make a difference if we cache only (or as a level 2 cache) the date and time without seconds that changes rarely?
I don't think so.
From my point of view, this would complicate code for a very minor perf gain, if any at all (depends on implementation, but if we split timestamp str into two parts, it will require additional '%s' and this will add an overhead for vfprintf() used under the hood, and this will affect every debug line).
But most important, all those patches only matter for a case when SSSD logs a **lot**, say hundreds lines per second. In this case timestamp re-creation once per second means <1% of debug lines and thus very low perf impact.
"""
See the full comment at https://github.com/SSSD/sssd/pull/5525#issuecomment-789658618
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
alexey-tikhonov commented: """
From my point of view, this would complicate code for a very minor perf gain
But the way, frankly speaking more or less the same statement applies to a second patch (replacement of `gettimeofday()`). I just think this doesn't complicate code that much so is ok to include (and impact can be more visible for in-mem logging case). """
See the full comment at https://github.com/SSSD/sssd/pull/5525#issuecomment-789684412
URL: https://github.com/SSSD/sssd/pull/5525 Author: alexey-tikhonov Title: #5525: Log performance improvements. Action: synchronized
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5525/head:pr5525 git checkout pr5525
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
alexey-tikhonov commented: """ UPD: changed sizeof `last_time_str` to 128 to prevent warning: ``` ../src/util/debug.c:314:13: note: ‘snprintf’ output between 18 and 73 bytes into a destination of size 32 ``` """
See the full comment at https://github.com/SSSD/sssd/pull/5525#issuecomment-790063021
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
thalman commented: """ Thank you for the patch. ACK """
See the full comment at https://github.com/SSSD/sssd/pull/5525#issuecomment-790670807
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
Label: -Waiting for review
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
Label: +Accepted
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
pbrezina commented: """ Ack as well. """
See the full comment at https://github.com/SSSD/sssd/pull/5525#issuecomment-791358035
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
Label: +Ready to push
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
pbrezina commented: """ Pushed PR: https://github.com/SSSD/sssd/pull/5525
* `master` * 5f840192eef2766db6642a3c9fd8b4f13be7222d - DEBUG: cache string representation of last timestamp * f553b57dd9d2d6088c58a9fe5b58b1d47a32d025 - DEBUG: replace gettimeofday() with time() if usec isn't needed * 1724482caeeb56e38a93190448a1f6e70a2dd3e3 - DEBUG: replace localtime() with localtime_r()
"""
See the full comment at https://github.com/SSSD/sssd/pull/5525#issuecomment-791359679
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
Label: +Pushed
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
Label: -Accepted
URL: https://github.com/SSSD/sssd/pull/5525 Title: #5525: Log performance improvements.
Label: -Ready to push
URL: https://github.com/SSSD/sssd/pull/5525 Author: alexey-tikhonov Title: #5525: Log performance improvements. Action: closed
To pull the PR as Git branch: git remote add ghsssd https://github.com/SSSD/sssd git fetch ghsssd pull/5525/head:pr5525 git checkout pr5525
sssd-devel@lists.fedorahosted.org
-
alexey-tikhonov -
pbrezina
-
thalman