[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/3] log: improve performance of qemu_log and qe
From: |
Alex Bennée |
Subject: |
Re: [Qemu-devel] [PATCH 1/3] log: improve performance of qemu_log and qemu_log_mask if disabled |
Date: |
Thu, 15 Oct 2015 19:36:57 +0100 |
User-agent: |
mu4e 0.9.13; emacs 24.5.50.4 |
Denis V. Lunev <address@hidden> writes:
> On 10/15/2015 08:23 PM, Alex Bennée wrote:
>> Denis V. Lunev <address@hidden> writes:
>>
>>> The patch is intended to avoid to perform any operation including
>>> calculation of log function arguments when the log is not enabled due to
>>> various reasons.
>>>
>>> Functions qemu_log and qemu_log_mask are replaced with variadic macros.
>>> Unfortunately the code is not C99 compatible and we can not use
>>> portable __VA_ARGS__ way. There are a lot of warnings in the other
>>> places with --std=c99. Thus the only way to achive the result is to use
>>> args.. GCC extension.
>>>
>>> Format checking performed by compiler will not suffer by this patch. It
>>> will be done inside in fprintf arguments checking.
>>>
>>> Signed-off-by: Denis V. Lunev <address@hidden>
>>> Signed-off-by: Pavel Butsykin <address@hidden>
>>> CC: Markus Armbruster <address@hidden>
>>> CC: Luiz Capitulino <address@hidden>
>>> CC: Eric Blake <address@hidden>
>>> CC: Peter Maydell <address@hidden>
>>> ---
>>> include/qemu/log.h | 17 ++++++++++++++---
>>> qemu-log.c | 21 ---------------------
>>> 2 files changed, 14 insertions(+), 24 deletions(-)
>>>
>>> diff --git a/include/qemu/log.h b/include/qemu/log.h
>>> index f880e66..57b8c96 100644
>>> --- a/include/qemu/log.h
>>> +++ b/include/qemu/log.h
>>> @@ -53,7 +53,13 @@ static inline bool qemu_loglevel_mask(int mask)
>>>
>>> /* main logging function
>>> */
>>> -void GCC_FMT_ATTR(1, 2) qemu_log(const char *fmt, ...);
>>> +#define qemu_log(args...) \
>>> + do { \
>>> + if (!qemu_log_enabled()) { \
>>> + break; \
>>> + } \
>>> + fprintf(qemu_logfile, args); \
>>> + } while (0)
>>>
>> I've had one of Peter's patches in my logging improvements queue for a
>> while although it uses a slightly different form which I prefer:
>>
>> -/* log only if a bit is set on the current loglevel mask
>> +/* log only if a bit is set on the current loglevel mask:
>> + * @mask: bit to check in the mask
>> + * @fmt: printf-style format string
>> + * @args: optional arguments for format string
>> */
>> -void GCC_FMT_ATTR(2, 3) qemu_log_mask(int mask, const char *fmt, ...);
>> -
>> +#define qemu_log_mask(MASK, FMT, ...) \
>> + do { \
>> + if (unlikely(qemu_loglevel_mask(MASK))) { \
>> + qemu_log(FMT, ## __VA_ARGS__); \
>> + } \
>>
>> See the message:
>>
>> qemu-log: Avoid function call for disabled qemu_log_mask logging
>
> as far as I can see that patch is one year old at least
> and my version is slightly better, it does the same for
> qemu_log.
Yeah it has been sitting in my queue for a while as part of a larger
series. I haven't been pushing it because the demand for the other
changes isn't super high.
Yes it would be better to do both qemu_log and qemu_log_mask.
> The difference is that old patch has unlikely() hint and does not
> contain break.
I'm not sure what the break form brings. It's personal preference I
guess but to me it is more natural to wrap things inside the condition
then use break to skip out of a block.
> I can revert the condition and add the hint in a couple
> of minutes if this will increase the chance to get
> both things merged. We should have both functions
> as macros.
>
> Will you accept that?
I'll certainly review the next submission. I also suggest you CC
qemu-trivial once you have collected all your r-b and a-b/s-o-b tags.
>
> Den
--
Alex Bennée
- Re: [Qemu-devel] What's the intended use of log.h logging? (was: [PATCH 2/3] log: report hmp/qmp command and qmp event), (continued)
[Qemu-devel] [PATCH 3/3] log: adds a timestamp to each log entry, Denis V. Lunev, 2015/10/15
[Qemu-devel] [PATCH 1/3] log: improve performance of qemu_log and qemu_log_mask if disabled, Denis V. Lunev, 2015/10/15
Re: [Qemu-devel] [PATCH 1/3] log: improve performance of qemu_log and qemu_log_mask if disabled, Markus Armbruster, 2015/10/16
Re: [Qemu-devel] [PATCH 0/3] QEMU logging improvements, Kashyap Chamarthy, 2015/10/15
Re: [Qemu-devel] [PATCH 0/3] QEMU logging improvements, Markus Armbruster, 2015/10/26