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

[Paolo Bonzini]

On 10/21/22 19: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.
>
> 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)
> +
>   static bool check_for_breakpoints(CPUState *cpu, target_ulong pc,
>                                     uint32_t *cflags)
>   {
>       CPUBreakpoint *bp;
>       bool match_page = false;
>   
> -    if (likely(QTAILQ_EMPTY(&cpu->breakpoints))) {
> -        return false;
> -    }
> -

It's a little more readable to just split out the slow path:

-static inline bool check_for_breakpoints(CPUState *cpu, target_ulong pc,
-                                         uint32_t *cflags)
+static bool check_for_breakpoints_slow(CPUState *cpu, target_ulong pc,
+                                       uint32_t *cflags)
 {
      CPUBreakpoint *bp;
      bool match_page = false;
  
-    if (likely(QTAILQ_EMPTY(&cpu->breakpoints))) {
-        return false;
-    }
     ...
 }
+
+static inline bool check_for_breakpoints(CPUState *cpu, target_ulong pc,
+                                         uint32_t *cflags)
+{
+    return unlikely(!QTAILQ_EMPTY(&cpu->breakpoints))
+        && check_for_breakpoints_slow(cpu, pc, cflags);
+}

Paolo