lkml.org 
[lkml]   [2016]   [Jul]   [12]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [RFC PATCH v5 0/7] perf config: Introduce default config key-value pairs arrays
    From
    Date
    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(-)
    >

    \
     
     \ /
      Last update: 2016-07-12 07:01    [W:3.122 / U:0.548 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site