git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Teng Long <dyroneteng@gmail.com>
To: git@jeffhostetler.com
Cc: avarab@gmail.com, derrickstolee@github.com, dyroneteng@gmail.com,
	git@vger.kernel.org, gitster@pobox.com, me@ttaylorr.com,
	tenglong.tl@alibaba-inc.com
Subject: Re: [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file
Date: Thu, 23 Jun 2022 17:38:36 +0800	[thread overview]
Message-ID: <20220623093836.113288-1-dyroneteng@gmail.com> (raw)
In-Reply-To: <78966fb1-d5be-2fb5-0f68-0fce0b93d249@jeffhostetler.com>

On Wed, 22 Jun 2022 08:51:18 -0400, Jeff Hostetler wrote:

> We should not be doing.  This would dump every repo-related
> boolean value on every command.

Yes. in v4 patch I use a new arg in repo_cfg_bool to decide
whether to trace or not, but after the above you mentioned, I
think it's still not a correct solution in my v4.

> I already have a GIT_TRACE2_CONFIG_PARAMS and trace2.configparams
> that will dump "interesting" config values to the trace2 log.
> Just set one of them to a list of regex's.  Look at the comment above
> trace2_cmd_list_config() in trace2.h for details.

That's fresh for me, thanks for telling me that.

I remove the changes of "repo_config_bool" from my v4 patch
then I try to know about "GIT_TRACE2_CONFIG_PARAMS" and
"trace2.configparams". Thereby, when I:

Execute "GIT_TRACE2_PERF=1 GIT_TRACE2_CONFIG_PARAMS=core.multipackIndex git rev-list  --test-bitmap HEAD"

15:21:38.812782 git.c:461                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
15:21:38.812797 git.c:461                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false

I checked my configs, I found if there exists multiple level configs.
it'll print multiple times. Like If I config all the global, system
and local on "core.multipackIndex=false" , the output will be:

15:41:50.614108 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
15:41:50.614123 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
15:41:50.614136 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false

And if I modified the local scope of core.multipackIndex to "true",
the output will be:

15:45:39.200172 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
15:45:39.200186 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:false
15:45:39.200200 git.c:462                    | d0 | main                     | def_param    |     |           |           |              | core.multipackindex:true

I'm not sure it's an intentional design or here should be only
print the final value that takes effect or should print all the
values if config multiple scopes on the same config.

Hence, I made a temporary patch below to try to add some
identifying information to know why we output these lines, like:


diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c
index c5c8cfbbaa..37a3163be1 100644
--- a/trace2/tr2_tgt_event.c
+++ b/trace2/tr2_tgt_event.c
@@ -479,9 +479,12 @@ static void fn_param_fl(const char *file, int line, const char *param,
 {
        const char *event_name = "def_param";
        struct json_writer jw = JSON_WRITER_INIT;
+       enum config_scope scope = current_config_scope();
+       const char *scope_name = config_scope_name(scope);
 
commit a089800b9dbc93a5dff9a46da7e66111e6e4343e (HEAD -> master, dyrone/master, dyrone/HEAD)
Author: Teng Long <dyroneteng@gmail.com>
Date:   Thu Jun 23 17:24:15 2022 +0800

    tr2: append scope info when same configs exist in multiple scopes
    
    Signed-off-by: Teng Long <dyroneteng@gmail.com>

diff --git a/trace2/tr2_tgt_event.c b/trace2/tr2_tgt_event.c
index c5c8cfbbaa..37a3163be1 100644
--- a/trace2/tr2_tgt_event.c
+++ b/trace2/tr2_tgt_event.c
@@ -479,9 +479,12 @@ static void fn_param_fl(const char *file, int line, const char *param,
 {
        const char *event_name = "def_param";
        struct json_writer jw = JSON_WRITER_INIT;
+       enum config_scope scope = current_config_scope();
+       const char *scope_name = config_scope_name(scope);
 
        struct json_writer jw = JSON_WRITER_INIT;
+       enum config_scope scope = current_config_scope();
+       const char *scope_name = config_scope_name(scope);
 
        jw_object_begin(&jw, 0);
        event_fmt_prepare(event_name, file, line, NULL, &jw);
+       jw_object_string(&jw, "scope", scope_name);
        jw_object_string(&jw, "param", param);
        jw_object_string(&jw, "value", value);
        jw_end(&jw);
diff --git a/trace2/tr2_tgt_normal.c b/trace2/tr2_tgt_normal.c
index c42fbade7f..e1b036a60c 100644
--- a/trace2/tr2_tgt_normal.c
+++ b/trace2/tr2_tgt_normal.c
@@ -298,8 +298,10 @@ static void fn_param_fl(const char *file, int line, const char *param,
                        const char *value)
 {
        struct strbuf buf_payload = STRBUF_INIT;
+       enum config_scope scope = current_config_scope();
+       const char *scope_name = config_scope_name(scope);
 
-       strbuf_addf(&buf_payload, "def_param %s=%s", param, value);
+       strbuf_addf(&buf_payload, "def_param scope:%s %s=%s", scope_name, param, value);
        normal_io_write_fl(file, line, &buf_payload);
        strbuf_release(&buf_payload);
 }
diff --git a/trace2/tr2_tgt_perf.c b/trace2/tr2_tgt_perf.c
index a1eff8bea3..c21bf8e651 100644
--- a/trace2/tr2_tgt_perf.c
+++ b/trace2/tr2_tgt_perf.c
@@ -441,10 +441,12 @@ static void fn_param_fl(const char *file, int line, const char *param,
 {
        const char *event_name = "def_param";
        struct strbuf buf_payload = STRBUF_INIT;
+       enum config_scope scope = current_config_scope();
+       const char *scope_name = config_scope_name(scope);
 
        strbuf_addf(&buf_payload, "%s:%s", param, value);
 
-       perf_io_write_fl(file, line, event_name, NULL, NULL, NULL, NULL,


----
The partial tr2 log looks like:

PERF:
17:00:35.933094 git.c:462                    | d0 | main                     | def_param    |     |           |           | system       | core.multipackindex:false
17:00:35.933110 git.c:462                    | d0 | main                     | def_param    |     |           |           | global       | core.multipackindex:false
17:00:35.933128 git.c:462                    | d0 | main                     | def_param    |     |           |           | local        | core.multipackindex:true

NORMAL:
17:14:32.905359 git.c:462                         def_param scope:system core.multipackindex=false
17:14:32.905370 git.c:462                         def_param scope:global core.multipackindex=false
17:14:32.905383 git.c:462                         def_param scope:local core.multipackindex=true

EVENT:
{"event":"def_param","sid":"20220623T092115.703660Z-H82fddc29-P0001812a","thread":"main","time":"2022-06-23T09:21:15.703920Z","file":"git.c","line":462,"scope":"system","param":"core.multipackindex","value":"false"}
{"event":"def_param","sid":"20220623T092115.703660Z-H82fddc29-P0001812a","thread":"main","time":"2022-06-23T09:21:15.703936Z","file":"git.c","line":462,"scope":"global","param":"core.multipackindex","value":"false"}
{"event":"def_param","sid":"20220623T092115.703660Z-H82fddc29-P0001812a","thread":"main","time":"2022-06-23T09:21:15.703952Z","file":"git.c","line":462,"scope":"local","param":"core.multipackindex","value":"true"}


> We also have GIT_TRACE2_ENV_VARS and trace2.envvars that will dump
> the values of "interesting" env vars.

> You can use these in your testing to log the config and env var
> values that you are interested in.

Wow, cool as GIT_TRACE2_CONFIG_PARAMS and trace2.configparams
you mentioned before. 

Thanks.

  reply	other threads:[~2022-06-23  9:39 UTC|newest]

Thread overview: 128+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-03-24 11:43 [PATCH v1 0/3] trace2 output for bitmap decision path Teng Long
2022-03-24 11:43 ` [PATCH v1 1/3] pack-bitmap.c: use "ret" in "open_midx_bitmap()" Teng Long
2022-03-24 19:11   ` Taylor Blau
2022-03-28  7:59     ` [PATCH v1 1/3] pack-bitmap.c: use "ret" in "open_midx_bitmap() Teng Long
2022-03-30  2:39       ` Taylor Blau
2022-03-24 11:44 ` [PATCH v1 2/3] pack-bitmap.c: add "break" statement in "open_pack_bitmap()" Teng Long
2022-03-24 18:40   ` Junio C Hamano
2022-03-24 19:06     ` Taylor Blau
2022-03-24 19:03   ` Taylor Blau
2022-03-29  2:49     ` Teng Long
2022-03-30  2:55       ` Taylor Blau
2022-03-30  7:32         ` Teng Long Teng Long
2022-03-30 13:17           ` Ævar Arnfjörð Bjarmason
2022-03-24 11:44 ` [PATCH v1 3/3] bitmap: add trace outputs during open "bitmap" file Teng Long
2022-03-24 18:42   ` Junio C Hamano
2022-03-24 13:22 ` [PATCH v1 0/3] trace2 output for bitmap decision path Ævar Arnfjörð Bjarmason
2022-03-29  7:38   ` Teng Long Teng Long
2022-03-29  8:54     ` Ævar Arnfjörð Bjarmason
2022-04-21 13:26 ` [PATCH v2 0/5] trace2 output for bitmap decision path Teng Long
2022-04-21 13:26   ` [PATCH v2 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-05-11 21:31     ` Taylor Blau
2022-04-21 13:26   ` [PATCH v2 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-05-11 21:31     ` Taylor Blau
2022-04-21 13:26   ` [PATCH v2 3/5] pack-bitmap.c: make warnings more detailed when opening bitmap Teng Long
2022-04-21 17:25     ` Taylor Blau
2022-05-06  9:08       ` Teng Long
2022-04-21 13:26   ` [PATCH v2 4/5] bitmap: add trace2 outputs during open "bitmap" file Teng Long
2022-04-21 15:51     ` Ævar Arnfjörð Bjarmason
2022-05-06 11:27       ` Teng Long
2022-05-06 11:53       ` Teng Long
2022-04-21 16:32     ` Jeff Hostetler
2022-05-06 12:43       ` Teng Long
2022-05-10 20:47         ` Jeff Hostetler
2022-04-21 13:26   ` [PATCH v2 5/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-04-21 15:41     ` Ævar Arnfjörð Bjarmason
2022-05-06 12:55       ` Teng Long
2022-06-12  7:44   ` [PATCH v3 0/5] Teng Long
2022-06-12  7:44     ` [PATCH v3 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-06-12  7:44     ` [PATCH v3 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-06-12  7:44     ` [PATCH v3 3/5] pack-bitmap.c: make warnings support i18N when opening bitmap Teng Long
2022-06-12  7:44     ` [PATCH v3 4/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-06-14  1:15       ` Taylor Blau
2022-06-20 13:17         ` Teng Long
2022-06-12  7:44     ` [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file Teng Long
2022-06-13 20:59       ` Junio C Hamano
2022-06-20 13:32         ` Teng Long
2022-06-14  1:40       ` Taylor Blau
2022-06-21  6:58         ` Teng Long
2022-06-22 12:51       ` Jeff Hostetler
2022-06-23  9:38         ` Teng Long [this message]
2022-06-23 15:14           ` Jeff Hostetler
2022-06-24  8:27             ` [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" Teng Long
2022-06-21 13:25     ` [PATCH v3 0/5] trace2 output for bitmap decision path Teng Long
2022-06-21 13:25       ` [PATCH v3 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-06-21 13:25       ` [PATCH v3 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-06-21 13:25       ` [PATCH v3 3/5] pack-bitmap.c: make warnings support i18N when opening bitmap Teng Long
2022-06-21 13:25       ` [PATCH v3 4/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-06-21 13:25       ` [PATCH v3 5/5] bitmap: add trace2 outputs during open "bitmap" file Teng Long
2022-06-22 13:04         ` Jeff Hostetler
2022-06-22 15:12           ` Junio C Hamano
2022-06-28  8:17       ` [PATCH v5 0/5] tr2: avoid to print "interesting" config repeatedly Teng Long
2022-06-28  8:17         ` [PATCH v5 1/5] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-06-28  8:17         ` [PATCH v5 2/5] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-06-28  8:17         ` [PATCH v5 3/5] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-06-28 18:04           ` Junio C Hamano
2022-07-05  9:04             ` Teng Long
2022-07-05 18:23               ` Junio C Hamano
2022-06-28  8:17         ` [PATCH v5 4/5] pack-bitmap.c: retrieve missing i18n translations Teng Long
2022-06-28  8:58           ` Ævar Arnfjörð Bjarmason
2022-06-28 17:28             ` Eric Sunshine
2022-07-06 14:19               ` Teng Long
2022-07-06 14:06             ` Teng Long
2022-06-28 18:07           ` Junio C Hamano
2022-07-07 11:59             ` Teng Long
2022-07-07 16:45               ` Junio C Hamano
2022-07-11 11:04                 ` Teng Long
2022-06-28  8:17         ` [PATCH v5 5/5] tr2: avoid to print "interesting" config repeatedly Teng Long
2022-06-28  9:13           ` Ævar Arnfjörð Bjarmason
2022-06-28 18:12             ` Junio C Hamano
2022-07-01 14:31               ` Jeff Hostetler
2022-07-11  4:11                 ` Teng Long
2022-07-11  3:51             ` Teng Long
2022-07-11 12:43         ` [PATCH v6 0/7] trace2: dump scope when print "interesting" config Teng Long
2022-07-11 12:43           ` [PATCH v6 1/7] clean: fixed issues related to text output format Teng Long
2022-07-11 21:08             ` Junio C Hamano
2022-07-13 11:44               ` Teng Long
2022-07-11 12:43           ` [PATCH v6 2/7] pack-bitmap.c: mark more strings for translations Teng Long
2022-07-11 12:43           ` [PATCH v6 3/7] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-07-11 12:44           ` [PATCH v6 4/7] pack-bitmap.c: don't ignore ENOENT silently Teng Long
2022-07-11 14:38             ` Ævar Arnfjörð Bjarmason
2022-07-13 14:14               ` Teng Long
2022-07-11 21:22             ` Junio C Hamano
2022-07-14 15:25               ` Teng Long
2022-07-11 12:44           ` [PATCH v6 5/7] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-07-11 14:53             ` Ævar Arnfjörð Bjarmason
2022-07-15  2:34               ` Teng Long
2022-07-11 12:44           ` [PATCH v6 6/7] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-07-11 12:44           ` [PATCH v6 7/7] tr2: dump names if config exist in multiple scopes Teng Long
2022-07-11 14:40             ` Ævar Arnfjörð Bjarmason
2022-07-11 19:19             ` Jeff Hostetler
2022-07-11 14:59           ` [PATCH v6 0/7] trace2: dump scope when print "interesting" config Ævar Arnfjörð Bjarmason
2022-07-18  8:36             ` Teng Long
2022-07-18 16:45           ` [PATCH v7 " Teng Long
2022-07-18 16:46             ` [PATCH v7 1/7] pack-bitmap.c: fix formatting of error messages Teng Long
2022-07-18 16:46             ` [PATCH v7 2/7] pack-bitmap.c: mark more strings for translations Teng Long
2022-07-18 16:46             ` [PATCH v7 3/7] pack-bitmap.c: rename "idx_name" to "bitmap_name" Teng Long
2022-07-18 16:46             ` [PATCH v7 4/7] pack-bitmap.c: do not ignore error when opening a bitmap file Teng Long
2022-07-18 16:46             ` [PATCH v7 5/7] pack-bitmap.c: using error() instead of silently returning -1 Teng Long
2022-07-18 16:46             ` [PATCH v7 6/7] pack-bitmap.c: continue looping when first MIDX bitmap is found Teng Long
2022-07-18 16:46             ` [PATCH v7 7/7] tr2: dump names if config exist in multiple scopes Teng Long
2022-07-18 20:13               ` Jeff Hostetler
2022-07-19  7:40                 ` tenglong.tl
2022-07-19 21:03               ` Junio C Hamano
2022-07-20 12:48                 ` tenglong.tl
2022-07-18 18:57             ` [PATCH v7 0/7] trace2: dump scope when print "interesting" config Junio C Hamano
2022-07-18 19:07               ` Ævar Arnfjörð Bjarmason
2022-07-19 11:26                 ` tenglong.tl
2022-07-19 11:42                   ` Ævar Arnfjörð Bjarmason
2022-07-19 12:34                     ` tenglong.tl
2022-07-21  9:05             ` [PATCH v8 0/6] pack-bitmap.c: optimize error messages tenglong.tl
2022-07-21  9:05               ` [PATCH v8 1/6] pack-bitmap.c: fix formatting of " tenglong.tl
2022-07-21  9:05               ` [PATCH v8 2/6] pack-bitmap.c: mark more strings for translations tenglong.tl
2022-07-21  9:05               ` [PATCH v8 3/6] pack-bitmap.c: rename "idx_name" to "bitmap_name" tenglong.tl
2022-07-21  9:05               ` [PATCH v8 4/6] pack-bitmap.c: do not ignore error when opening a bitmap file tenglong.tl
2022-07-21  9:05               ` [PATCH v8 5/6] pack-bitmap.c: using error() instead of silently returning -1 tenglong.tl
2022-07-21  9:05               ` [PATCH v8 6/6] pack-bitmap.c: continue looping when first MIDX bitmap is found tenglong.tl
2022-07-21 23:01               ` [PATCH v8 0/6] pack-bitmap.c: optimize error messages Junio C Hamano
2022-07-22  6:17                 ` tenglong.tl

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20220623093836.113288-1-dyroneteng@gmail.com \
    --to=dyroneteng@gmail.com \
    --cc=avarab@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@jeffhostetler.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=tenglong.tl@alibaba-inc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).