Attention is currently required from: Hoernchen.
neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/libosmocore/+/30610 )

Change subject: logging: rework the macros
......................................................................


Patch Set 1: Code-Review-2

(2 comments)

File include/osmocom/core/logging.h:

https://gerrit.osmocom.org/c/libosmocore/+/30610/comment/3b84c434_dcbbf700
PS1, Line 64:           if (log_check_level(ss, level)) \
it is a hugely important performance decision to do log_check_level() in the 
macro, *before* passing the varargs to logp2(). That is the entire point why 
log_check_level() was introduced in this way: if no logging target will print 
this log message, we can skip all of the string composition for the log message 
entirely.

For example, in this:

  LOGP(DMAIN, LOGL_DEBUG, "data: %s\n", osmo_hexdump(data, len));

we skip calling osmo_hexdump() when DMAIN's DEBUG is not enabled anywhere.

With your code changes, we would first call osmo_hexdump(), pass the result to 
logp2_with_check(), and only then discard the log message. That would be a 
massive performance dip for loaded sites where DEBUG logging is switched off.

Now, because log_check_level() is called first, we need to check whether 
logging was initialized and redirect to logp_stub() even before that -- 
log_check_level() just aborts the program, with assert(osmo_log_info)


File src/logging.c:

https://gerrit.osmocom.org/c/libosmocore/+/30610/comment/afc179d9_a081d87a
PS1, Line 87: struct log_info *osmo_log_info;
(undoing part of earlier patch)



--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/30610
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I03efa954cb9e991d2c3da4b61b12aac651e0efa2
Gerrit-Change-Number: 30610
Gerrit-PatchSet: 1
Gerrit-Owner: Hoernchen <ew...@sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de>
Gerrit-Reviewer: neels <nhofm...@sysmocom.de>
Gerrit-Attention: Hoernchen <ew...@sysmocom.de>
Gerrit-Comment-Date: Thu, 15 Dec 2022 02:32:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Reply via email to