[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v9 04/13] accel: replacing part of CONFIG_PROFILER with TBSta
From: |
Richard Henderson |
Subject: |
Re: [PATCH v9 04/13] accel: replacing part of CONFIG_PROFILER with TBStats |
Date: |
Tue, 8 Oct 2019 09:58:55 -0400 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 |
On 10/7/19 11:28 AM, Alex Bennée wrote:
> +struct jit_profile_info {
> + uint64_t translations;
> + uint64_t aborted;
> + uint64_t ops;
> + unsigned ops_max;
> + uint64_t del_ops;
> + uint64_t temps;
> + unsigned temps_max;
> + uint64_t host;
> + uint64_t guest;
> + uint64_t search_data;
> +};
Hmm. Maybe better to define a single structure that you can share between the
collection here and the storage in TCGProfile?
Also, "host" and "guest" are not helpful names...
> + jpi->host += tbs->code.out_len;
> + jpi->guest += tbs->code.in_len;
... code_int_len and code_out_len are helpful names.
> @@ -1807,6 +1805,7 @@ TranslationBlock *tb_gen_code(CPUState *cpu,
> tb->tb_stats = NULL;
> }
>
> +
> tcg_func_start(tcg_ctx);
>
> tcg_ctx->cpu = env_cpu(env);
Watch the random whitespace.
> +#define stat_per_translation(stat, name) \
> + (stat->translations.total ? stat->name / stat->translations.total : 0)
Does this really need to go in the header?
> +static void collect_jit_profile_info(void *p, uint32_t hash, void *userp)
> +{
> + struct jit_profile_info *jpi = userp;
> + TBStatistics *tbs = p;
> +
> + jpi->translations += tbs->translations.total;
> + jpi->ops += tbs->code.num_tcg_ops;
> + if (stat_per_translation(tbs, code.num_tcg_ops) > jpi->ops_max) {
> + jpi->ops_max = stat_per_translation(tbs, code.num_tcg_ops);
> + }
> + jpi->del_ops += tbs->code.deleted_ops;
> + jpi->temps += tbs->code.temps;
> + if (stat_per_translation(tbs, code.temps) > jpi->temps_max) {
> + jpi->temps_max = stat_per_translation(tbs, code.temps);
> + }
> + jpi->host += tbs->code.out_len;
> + jpi->guest += tbs->code.in_len;
> + jpi->search_data += tbs->code.search_out_len;
> +}
E.g.
unsigned long total = tbs->translations.total;
if (total) {
unsigned tmp;
tmp = tbs->code.num_tcg_ops / total;
jpi->ops_max = MAX(jpi->ops_max, tmp);
tmp = tbs->code.temps / total;
jpi->temps_max = MAX(jpi->temps_max, tmp);
}
It does occur to me to wonder why code.num_guest_inst etc are "unsigned" while
translations.total are "unsigned long", given that they are both accumulations
over many TBs.
r~
- [PATCH v9 03/13] accel: collecting JIT statistics, (continued)
- [PATCH v9 03/13] accel: collecting JIT statistics, Alex Bennée, 2019/10/07
- [PATCH v9 01/13] accel/tcg: introduce TBStatistics structure, Alex Bennée, 2019/10/07
- [PATCH v9 06/13] debug: add -d tb_stats to control TBStatistics collection:, Alex Bennée, 2019/10/07
- [PATCH v9 07/13] monitor: adding tb_stats hmp command, Alex Bennée, 2019/10/07
- [PATCH v9 04/13] accel: replacing part of CONFIG_PROFILER with TBStats, Alex Bennée, 2019/10/07
- Re: [PATCH v9 04/13] accel: replacing part of CONFIG_PROFILER with TBStats,
Richard Henderson <=
- [PATCH v9 08/13] tb-stats: reset the tracked TBs on a tb_flush, Alex Bennée, 2019/10/07
- [PATCH v9 10/13] tb-stats: dump hot TBs at the end of the execution, Alex Bennée, 2019/10/07
- [PATCH v9 05/13] accel: adding TB_JIT_TIME and full replacing CONFIG_PROFILER, Alex Bennée, 2019/10/07
- [PATCH v9 12/13] tb-stats: adding TBStatistics info into perf dump, Alex Bennée, 2019/10/07
- [PATCH v9 13/13] configure: remove the final bits of --profiler support, Alex Bennée, 2019/10/07