This is merely a historical archive of years 2008-2021, before the migration to mailman3.
A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/OpenBSC@lists.osmocom.org/.
Holger Freyther holger at freyther.de
> On 21 Dec 2015, at 15:13, Jacob Erlbeck <jerlbeck at sysmocom.de> wrote:
>
> Hi Holger,
>
> On 21.12.2015 14:17, Holger Freyther wrote:
>>
>>> On 17 Nov 2015, at 11:52, Jacob Erlbeck <jerlbeck at 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