Re: [PATCH 1/3] accel/tcg: Add a quicker check for breakpoints

[Richard Henderson]

On 10/22/22 03:01, Leandro Lupori wrote:
> Profiling QEMU during Fedora 35 for PPC64 boot revealed that a
> considerable amount of time was being spent in
> check_for_breakpoints() (0.61% of total time on PPC64 and 2.19% on
> amd64), even though it was just checking that its queue was empty
> and returning, when no breakpoints were set. It turns out this
> function is not inlined by the compiler and it's always called by
> helper_lookup_tb_ptr(), one of the most called functions.
>
> By moving the check for empty queue to the have_breakpoints()
> macro and calling check_for_breakpoints() only when it returns
> true, it's possible to avoid the call overhead. An improvement of
> about 3% in total time was measured on POWER9.

Wow, 3%?

> Signed-off-by: Leandro Lupori <leandro.lupori@eldorado.org.br>
> ---
>   accel/tcg/cpu-exec.c | 13 +++++++------
>   1 file changed, 7 insertions(+), 6 deletions(-)
>
> diff --git a/accel/tcg/cpu-exec.c b/accel/tcg/cpu-exec.c
> index f9e5cc9ba0..9eec01ad9a 100644
> --- a/accel/tcg/cpu-exec.c
> +++ b/accel/tcg/cpu-exec.c
> @@ -304,16 +304,15 @@ static void log_cpu_exec(target_ulong pc, CPUState *cpu,
>       }
>   }
>   
> +#define have_breakpoints(cpu)   (likely(QTAILQ_EMPTY(&(cpu)->breakpoints)) ? \
> +                                 false : true)

First, always avoid useless ?:.

Second, I think renaming the existing check_for_breakpoints to check_for_breakpoints_slow 
and make this test be an inline function instead.  E.g.

static bool check_for_breakpoints(...)
{
    if (unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))) {
        return check_for_breakpoints_slow(...);
    }
    return false;
}


r~