On 21 Dec 2015, at 15:13, Jacob Erlbeck
<jerlbeck(a)sysmocom.de> wrote:
Hi Holger,
On 21.12.2015 14:17, Holger Freyther wrote:
> On 17 Nov 2015, at 11:52, Jacob Erlbeck <jerlbeck(a)sysmocom.de> wrote:
+ /*
This might get logged (ignoring filters) */
What is the reasoning for this part?
I wanted to have a weaker criterion than 'return true if and only if
something would be logged' to be more flexible when it comes to
optimization. I had lookup tables in mind which cache the max level per
facility, and which get updated when e.g. log_set_log_level is called.
This would be O(1) consuming just a few CPU cycles per log, but would
not help when filtering is used.
And I didn't really understand filters when I wrote that commit.
Including them into the log_check_level decision definitely makes sense
if I want to use log DEBUG level stuff on an embedded or high load system.
Okay maybe I am missing a case here but it feels like we don't save anything
by not calling it. So if the log level is good for output we return 1 and then do
the full work. It doesn't really matter which function we do this work in.
And as this doesn't matter we can leave it out. I posted a patch that removes
the selection criteria and subsys code clone.
The below does
not work yet but shows the general idea. If the check routine
already finds the first target that will generate an output, then let us use it? This
is why I have put an output parameter in there and pass it a long.
Reusing the target pointer for logging seems like a nice feature, but it
bloats the API and only gains anything, if something gets actually
logged which is probably much more expensive than rescanning the
probably very few targets that came in front and are in the CPU cache
already anyway. So I wouldn't do that optimization unless profiling
would tell me to do so.
I consider LOGP, LOGPC, DEBUGP, DEBUGPC as the API. I don't think we have
any direct callers of osmo_vlogp, logp, logp2. The change to start from the target
that has been found is trivial and if we can avoid doing work without increasing the
memory usage then we should do it.
We do not have a libsomocore.map but if we do I would not export logp, logp2,
osmo_vlogp and the log_check_level functions.
I thought of using log_check_level explicitly in user
code, too. E.g. if
a log statement would require some preparation that can be done at
argument position. That gets a little clumsy with that additional parameter.
I did some profiling with the CPU and the current log_check_level was
way below the few remaining calls to logp.
The thing that is not pretty is how to print
"tar" and everything that follows it,
naturally I would like to use a do {} while() loop but not roll the llist_entry by
hand (or maybe I do).
It is no look-up table but we avoid to duplicate the selection criteria and to
iterate over targets (the one or two) we already have looked at?
I guess that a) not ignoring the filter and b) doing a O(1) pre-check
(possibly inlined) will gain us more.
I don't think we want to inline all pre-checks (list traversal). Your patch helps
to avoid calling osmo_hexdump, gsm_lchan_name and others. I remove the
code clones and continue (yeah redo the integer compares and a load) from
where log_check_level stopped. It is a sound approach.
cheers
holger