Re: [RFC PATCH v5 0/7] perf config: Introduce default config key-value pairs arrays

From: Taeung Song
Date: Tue Jul 12 2016 - 00:20:45 EST


Hi, Namhyung :)

As you know, the patch work from v3 to v4 took too long time
because of a patchset refactoring perf_config().

So I guess it is hard for you to recall this patchset for new default
config arrays but could you a bit response two simple questions ?


1) I renamed 'fb_ground' to 'fore_back_colors' in struct ui_browser_colorset at ui/browser.c.

Is it better than before ?

2) I added not only 'struct default_config_item' but also
'struct default_config_section'.
(In order to use const type and 'const struct default_config_item *items'
instead of using 'struct list_head items')

could you agree this way ?


Thanks,
Taeung

On 07/06/2016 02:20 PM, Taeung Song wrote:
Hello, :)

When initializing default perf config values,
we currently use values of actual type(int, bool, char *, etc.).
But I suggest using default config key-value pairs arrays.

For example,
If there isn't user config value at ~/.perfconfig for 'annotate.use_offset' config variable,
default value for it is 'true' bool type value in perf like below.

At ui/browsers/annoate.c

static struct annotate_browser_opt {
bool hide_src_code,
use_offset,
jump_arrows,
show_linenr,
show_nr_jumps,
show_total_period;
} annotate_browser__opts = {
.use_offset = true,
.jump_arrows = true,
};

But if we use new config arrays that have all default config key-value pairs,
we could initialize default config values with them.

If we do, we can manage default perf config values at one spot (like util/config.c)
and It can be easy and simple to modify default config values or add new configs.

For example,
If we use new default config arrays and there isn't user config value for 'annoate.use_offset'
default value for it will be set as annotate_config_items[CONFIG_ANNOATE_USE_OFFSET].value
instead of actual boolean type value 'true'.

IMHO, I think it would needed to use new default config arrays
to manage default perf config values more effectively.
And this pathset contains patchs for only 'colors' and 'annoate' section
because waiting for other opinions.

If you review this patchset, I'd appreciate it :-)

Thanks,
Taeung

v5:
- rebased on current acme/perf/core

v4:
- rename 'fb_ground' to 'fore_back_colors' (Namhyung)
- add struct default_config_section
- split first patch[PATCH 1/7] as two
- remove perf_default_config_init() at perf.c
- rebased on current acme/perf/core

v3:
- remove default config arrays for the rest sections except 'colors' and 'annotate'
- use combined {fore, back}ground colors instead of each two color
- introduce perf_default_config_init() that call all default_*_config_init()
for each config section

v2:
- rename 'ui_browser__config_gcolors' to 'ui_browser__config_colors' (Arnaldo)
- change 'ground colors' to '{back, fore}ground colors' (Arnaldo)
- use strtok + ltrim instead of strchr and while (isspace(*++bg)); (Arnaldo)

Taeung Song (7):
perf config: Introduce default_config_section and default_config_item
for default config key-value pairs
perf config: Add macros assigning key-value pairs to
default_config_item
perf config: Add 'colors' section default configs arrrays
perf config: Use combined {fore,back}ground colors value instead of
each two color
perf config: Initialize ui_browser__colorsets with default config
items
perf config: Add 'annotate' section default configs arrrays
perf config: Initialize annotate_browser__opts with default config
items

tools/perf/ui/browser.c | 64 +++++++++++++++++-------------
tools/perf/ui/browsers/annotate.c | 16 ++++++--
tools/perf/util/config.c | 26 +++++++++++++
tools/perf/util/config.h | 82 +++++++++++++++++++++++++++++++++++++++
4 files changed, 156 insertions(+), 32 deletions(-)