Tanay Abhra <tanayabh@xxxxxxxxx> writes:
> Of all the functions in `git_config*()` family, `git_config()` has the
> most invocations in the whole code base. Each `git_config()` invocation
> causes config file rereads which can be avoided using the config-set API.
>
> Use the config-set API to rewrite `git_config()` to use the config caching
> layer to avoid config file rereads on each invocation during a git process
> lifetime. First invocation constructs the cache, and after that for each
> successive invocation, `git_config()` feeds values from the config cache
> instead of rereading the configuration files.
>
> Signed-off-by: Tanay Abhra <tanayabh@xxxxxxxxx>
> ---
> config.c | 29 +++++++++++++++++++++++++++--
> 1 file changed, 27 insertions(+), 2 deletions(-)
This is the natural logical conclusion ;-)
> diff --git a/config.c b/config.c
> index 6db8f97..a7fb9a4 100644
> --- a/config.c
> +++ b/config.c
> @@ -1232,11 +1232,36 @@ int git_config_with_options(config_fn_t fn, void *data,
> return ret;
> }
>
> -int git_config(config_fn_t fn, void *data)
> +static int git_config_raw(config_fn_t fn, void *data)
> {
> return git_config_with_options(fn, data, NULL, 1);
> }
>
> +static int configset_iter(struct config_set *cs, config_fn_t fn, void *data)
> +{
> + int i;
> + struct string_list *strptr;
We know string_list would hold strings, so naming the variable
strptr does not give us much information. As this is a list of
values you would get by querying for a variable (or "key"), perhaps
name it "values" or something?
> + struct config_set_element *entry;
> + struct hashmap_iter iter;
Have a blank line between the local variable definitions (above) and
the first executable statement (below); it would make it easier to
read, especially because out codebase do not have decl-after-stmt.
> + hashmap_iter_init(&cs->config_hash, &iter);
> + while ((entry = hashmap_iter_next(&iter))) {
> + strptr = &entry->value_list;
> + for (i = 0; i < strptr->nr; i++) {
> + if (fn(entry->key, strptr->items[i].string, data) < 0)
> + die("bad config file line in (TODO: file/line info)");
> + }
> + }
> + return 0;
> +}
> +
> +static void git_config_check_init(void);
> +
> +int git_config(config_fn_t fn, void *data)
> +{
> + git_config_check_init();
> + return configset_iter(&the_config_set, fn, data);
> +}
Perhaps if you define this function at the right place in the file
you do not have to add an forward decl of git_config_check_init()?
> static struct config_set_element *configset_find_element(struct config_set *cs, const char *key)
> {
> struct config_set_element k;
> @@ -1418,7 +1443,7 @@ static void git_config_check_init(void)
> if (the_config_set.hash_initialized)
> return;
> git_configset_init(&the_config_set);
> - git_config(config_set_callback, &the_config_set);
> + git_config_raw(config_set_callback, &the_config_set);
> }
>
> void git_config_clear(void)
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html