[RFC,19/21] plugins: add an example hotblocks plugin

This plugin attempts to track hotblocks by total execution count and
average time to return to block. It is intended to be lower impact
than saving a long log file and post-processing it.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
 trace/plugins/hotblocks/hotblocks.c | 69 +++++++++++++++++++++++++++++
 1 file changed, 69 insertions(+)
 create mode 100644 trace/plugins/hotblocks/hotblocks.c

I guess this one is too tcg-dependent.
It count TB's, but breaking code into TBs may depend on many things,
like breakpoints, record/replay, ...

I mean that this measuring approach may be used only in some specific
cases, and not ok as an example.

Pavel Dovgalyuk

> -----Original Message-----
> From: Alex Bennée [mailto:alex.bennee@linaro.org]
> Sent: Friday, October 05, 2018 6:49 PM
> To: qemu-devel@nongnu.org
> Cc: Pavel.Dovgaluk@ispras.ru; vilanova@ac.upc.edu; cota@braap.org; Alex Bennée; Stefan
> Hajnoczi
> Subject: [RFC PATCH 19/21] plugins: add an example hotblocks plugin
> 
> This plugin attempts to track hotblocks by total execution count and
> average time to return to block. It is intended to be lower impact
> than saving a long log file and post-processing it.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> ---
>  trace/plugins/hotblocks/hotblocks.c | 69 +++++++++++++++++++++++++++++
>  1 file changed, 69 insertions(+)
>  create mode 100644 trace/plugins/hotblocks/hotblocks.c
> 
> diff --git a/trace/plugins/hotblocks/hotblocks.c b/trace/plugins/hotblocks/hotblocks.c
> new file mode 100644
> index 0000000000..e9166ad1a5
> --- /dev/null
> +++ b/trace/plugins/hotblocks/hotblocks.c
> @@ -0,0 +1,69 @@
> +/*
> + * Execution Hotblocks Plugin
> + *
> + * Copyright (c) 2018
> + * Written by Alex Bennée <alex.bennee@linaro.org>
> + *
> + * This code is licensed under the GNU GPL v2.
> + */
> +
> +#include <stdint.h>
> +#include <stdio.h>
> +#include <glib.h>
> +#include <time.h>
> +#include "plugins.h"
> +
> +/* Plugins need to take care of their own locking */
> +GMutex lock;
> +GHashTable *hotblocks;
> +
> +typedef struct {
> +    uintptr_t pc;
> +    unsigned int hits;
> +    struct timespec last;
> +    unsigned long total_time;
> +} ExecCount;
> +
> +bool plugin_init(const char *args)
> +{
> +    hotblocks = g_hash_table_new(NULL, g_direct_equal);
> +    return true;
> +}
> +
> +char *plugin_status(void)
> +{
> +    GString *report = g_string_new("We have ");
> +    char *r;
> +    g_mutex_lock(&lock);
> +    g_string_append_printf(report, "%ud entries in the hash table\n",
> +                           g_hash_table_size(hotblocks));
> +    g_mutex_unlock(&lock);
> +    r = report->str;
> +    g_string_free(report, FALSE);
> +    return r;
> +}
> +
> +bool exec_tb(void *tb, uintptr_t pc)
> +{
> +    ExecCount *cnt;
> +    struct timespec current;
> +    clock_gettime(CLOCK_MONOTONIC, &current);
> +
> +    g_mutex_lock(&lock);
> +    cnt = (ExecCount *) g_hash_table_lookup(hotblocks, (gconstpointer) pc);
> +    if (cnt) {
> +        cnt->hits++;
> +        cnt->total_time += current.tv_nsec - cnt->last.tv_nsec;
> +        cnt->last = current;
> +    } else {
> +        cnt = g_new0(ExecCount, 1);
> +        cnt->pc = pc;
> +        cnt->last = current;
> +        cnt->hits = 1;
> +        g_hash_table_insert(hotblocks, (gpointer) pc, (gpointer) cnt);
> +    }
> +    g_mutex_unlock(&lock);
> +
> +    /* As we are collecting up samples no reason to continue tracing */
> +    return false;
> +}
> --
> 2.17.1

Pavel Dovgalyuk <dovgaluk@ispras.ru> writes:

> I guess this one is too tcg-dependent.
> It count TB's, but breaking code into TBs may depend on many things,
> like breakpoints, record/replay, ...
>
> I mean that this measuring approach may be used only in some specific
> cases, and not ok as an example.

You are right it involves a degree of knowledge of how the TCG works
although it doesn't look inside the internals to do it - you do have to
specify -d nochain to get useful numbers.

I used this as an example in lieu of defining tracepoints in the main
translation loop. They would be better there as it would be unambiguous
what the behaviour would be. That said I think a translation block start
as well as pre/post instruction translation points would be useful as
block level granularity will be good enough for a number of use cases.

>
> Pavel Dovgalyuk
>
>> -----Original Message-----
>> From: Alex Bennée [mailto:alex.bennee@linaro.org]
>> Sent: Friday, October 05, 2018 6:49 PM
>> To: qemu-devel@nongnu.org
>> Cc: Pavel.Dovgaluk@ispras.ru; vilanova@ac.upc.edu; cota@braap.org; Alex Bennée; Stefan
>> Hajnoczi
>> Subject: [RFC PATCH 19/21] plugins: add an example hotblocks plugin
>>
>> This plugin attempts to track hotblocks by total execution count and
>> average time to return to block. It is intended to be lower impact
>> than saving a long log file and post-processing it.
>>
>> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
>> ---
>>  trace/plugins/hotblocks/hotblocks.c | 69 +++++++++++++++++++++++++++++
>>  1 file changed, 69 insertions(+)
>>  create mode 100644 trace/plugins/hotblocks/hotblocks.c
>>
>> diff --git a/trace/plugins/hotblocks/hotblocks.c b/trace/plugins/hotblocks/hotblocks.c
>> new file mode 100644
>> index 0000000000..e9166ad1a5
>> --- /dev/null
>> +++ b/trace/plugins/hotblocks/hotblocks.c
>> @@ -0,0 +1,69 @@
>> +/*
>> + * Execution Hotblocks Plugin
>> + *
>> + * Copyright (c) 2018
>> + * Written by Alex Bennée <alex.bennee@linaro.org>
>> + *
>> + * This code is licensed under the GNU GPL v2.
>> + */
>> +
>> +#include <stdint.h>
>> +#include <stdio.h>
>> +#include <glib.h>
>> +#include <time.h>
>> +#include "plugins.h"
>> +
>> +/* Plugins need to take care of their own locking */
>> +GMutex lock;
>> +GHashTable *hotblocks;
>> +
>> +typedef struct {
>> +    uintptr_t pc;
>> +    unsigned int hits;
>> +    struct timespec last;
>> +    unsigned long total_time;
>> +} ExecCount;
>> +
>> +bool plugin_init(const char *args)
>> +{
>> +    hotblocks = g_hash_table_new(NULL, g_direct_equal);
>> +    return true;
>> +}
>> +
>> +char *plugin_status(void)
>> +{
>> +    GString *report = g_string_new("We have ");
>> +    char *r;
>> +    g_mutex_lock(&lock);
>> +    g_string_append_printf(report, "%ud entries in the hash table\n",
>> +                           g_hash_table_size(hotblocks));
>> +    g_mutex_unlock(&lock);
>> +    r = report->str;
>> +    g_string_free(report, FALSE);
>> +    return r;
>> +}
>> +
>> +bool exec_tb(void *tb, uintptr_t pc)
>> +{
>> +    ExecCount *cnt;
>> +    struct timespec current;
>> +    clock_gettime(CLOCK_MONOTONIC, &current);
>> +
>> +    g_mutex_lock(&lock);
>> +    cnt = (ExecCount *) g_hash_table_lookup(hotblocks, (gconstpointer) pc);
>> +    if (cnt) {
>> +        cnt->hits++;
>> +        cnt->total_time += current.tv_nsec - cnt->last.tv_nsec;
>> +        cnt->last = current;
>> +    } else {
>> +        cnt = g_new0(ExecCount, 1);
>> +        cnt->pc = pc;
>> +        cnt->last = current;
>> +        cnt->hits = 1;
>> +        g_hash_table_insert(hotblocks, (gpointer) pc, (gpointer) cnt);
>> +    }
>> +    g_mutex_unlock(&lock);
>> +
>> +    /* As we are collecting up samples no reason to continue tracing */
>> +    return false;
>> +}
>> --
>> 2.17.1


--
Alex Bennée

On 10/5/18 8:49 AM, Alex Bennée wrote:
> +char *plugin_status(void)
> +{
> +    GString *report = g_string_new("We have ");
> +    char *r;
> +    g_mutex_lock(&lock);
> +    g_string_append_printf(report, "%ud entries in the hash table\n",
> +                           g_hash_table_size(hotblocks));
> +    g_mutex_unlock(&lock);
> +    r = report->str;
> +    g_string_free(report, FALSE);
> +    return r;
> +}

You're not reporting the entries themselves?
Also, are you sure you don't want to just return the GString?


r~

Richard Henderson <richard.henderson@linaro.org> writes:

> On 10/5/18 8:49 AM, Alex Bennée wrote:
>> +char *plugin_status(void)
>> +{
>> +    GString *report = g_string_new("We have ");
>> +    char *r;
>> +    g_mutex_lock(&lock);
>> +    g_string_append_printf(report, "%ud entries in the hash table\n",
>> +                           g_hash_table_size(hotblocks));
>> +    g_mutex_unlock(&lock);
>> +    r = report->str;
>> +    g_string_free(report, FALSE);
>> +    return r;
>> +}
>
> You're not reporting the entries themselves?
> Also, are you sure you don't want to just return the GString?

I considered it. I wondered if exposing a glib dependency to the plugins
was fair game? It would make things simpler and QEMU could even allocate
it for the plugin.

>
>
> r~


--
Alex Bennée