| Message ID | 32f8b9ae6bb6aff0ce55ee494c4c0d40c672752b.1658472474.git.dyroneteng@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tr2: shows the scope unconditionally with config | expand |
On Fri, Jul 22 2022, tenglong.tl wrote: > From: Teng Long <dyroneteng@gmail.com> > > It's supported to print "interesting" config key-value paire > to tr2 log by setting "GIT_TRACE2_CONFIG_PARAMS" environment > variable and the "trace2.configparam" config, let's add the > related docs in Documentaion/technical/api-trace2.txt. > > Signed-off-by: Teng Long <dyroneteng@gmail.com> > --- > Documentation/technical/api-trace2.txt | 32 ++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt > index 77a150b30e..ddc0bfb9c9 100644 > --- a/Documentation/technical/api-trace2.txt > +++ b/Documentation/technical/api-trace2.txt > @@ -717,6 +717,7 @@ The "exec_id" field is a command-unique id and is only useful if the > { > "event":"def_param", > ... > + "scope":"global", > "param":"core.abbrev", > "value":"7" > } > @@ -1207,6 +1208,37 @@ at offset 508. > This example also shows that thread names are assigned in a racy manner > as each thread starts and allocates TLS storage. > > +Print Configs:: > + > + Dump "interesting" config values to trace2 log. > ++ > +The environment variable `GIT_TRACE2_CONFIG_PARAMS` and configuration > +`trace2.configparams` can be used to output config values which you care > +about(see linkgit:git-config[1). For example: I didn't notice this before, but this is an addition to a long section where the examples are ------- delimited, starting with "in this example.." usually. So this "print configs" seems like on odd continuation. Shouldn't this copy the template of "Thread Events::" above. I.e. something like (I have not tried to asciidoc render this): Config (def param) Events:: We can optionally emit configuration events, see `trace2.configParams` in linkgit:git-config[1] for how to enable it. + < your example below would follow this> I.e. re my earlier mention of git-config we it explains GIT_TRACE2_CONFIG_PARAMS, so perhaps it suffices to just link to linkgit:git-config[1] for that. Also a nit: trace2.configParams, not trace2.configparams. > ++ > +---------------- > +$ git config color.ui auto > +---------------- > ++ > +Then, mark the config `color.ui` as "interesting" config with > +`GIT_TRACE2_CONFIG_PARAMS`: > ++ > +---------------- > +$ export GIT_TRACE2_PERF_BRIEF=1 > +$ export GIT_TRACE2_PERF=~/log.perf > +$ export GIT_TRACE2_CONFIG_PARAMS=color.ui > +$ git version > +... > +$ cat ~/log.perf > +d0 | main | version | | | | | ... > +d0 | main | start | | 0.001642 | | | /usr/local/bin/git version > +d0 | main | cmd_name | | | | | version (version) > +d0 | main | def_param | | | | | color.ui:auto > +d0 | main | data | r0 | 0.002100 | 0.002100 | fsync | fsync/writeout-only:0 > +d0 | main | data | r0 | 0.002126 | 0.002126 | fsync | fsync/hardware-flush:0 > +d0 | main | exit | | 0.002142 | | | code:0 > +d0 | main | atexit | | 0.002161 | | | code:0 > +---------------- > == Future Work > > === Relationship to the Existing Trace Api (api-trace.txt)
"tenglong.tl" <dyroneteng@gmail.com> writes: > From: Teng Long <dyroneteng@gmail.com> > > It's supported to print "interesting" config key-value paire > to tr2 log by setting "GIT_TRACE2_CONFIG_PARAMS" environment > variable and the "trace2.configparam" config, let's add the > related docs in Documentaion/technical/api-trace2.txt. > > Signed-off-by: Teng Long <dyroneteng@gmail.com> > --- > Documentation/technical/api-trace2.txt | 32 ++++++++++++++++++++++++++ > 1 file changed, 32 insertions(+) > > diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt > index 77a150b30e..ddc0bfb9c9 100644 > --- a/Documentation/technical/api-trace2.txt > +++ b/Documentation/technical/api-trace2.txt > @@ -717,6 +717,7 @@ The "exec_id" field is a command-unique id and is only useful if the > { > "event":"def_param", > ... > + "scope":"global", > "param":"core.abbrev", > "value":"7" > } Isn't this a bit premature to do in [PATCH 1/2]?
> Isn't this a bit premature to do in [PATCH 1/2]?
Yes, one oversight, sorry for that and will fix.
Thanks.
"tenglong.tl" <dyroneteng@gmail.com> writes: >> Isn't this a bit premature to do in [PATCH 1/2]? > > Yes, one oversight, sorry for that and will fix. I've tweaked with "rebase -i" after queuing and before pushing the integration result out, so if you can double check the result to make sure I didn't screw up, there is no need to resend. Thanks.
Junio C Hamano <gitster@pobox.com> writes: > I've tweaked with "rebase -i" after queuing and before pushing the > integration result out, so if you can double check the result to > make sure I didn't screw up, there is no need to resend. OK, got it. There is no inconsistency in understanding, I agree with your suggestion. Thanks for explanation.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > On Fri, Jul 22 2022, tenglong.tl wrote: > ... >> +Print Configs:: >> + >> + Dump "interesting" config values to trace2 log. >> ++ >> +The environment variable `GIT_TRACE2_CONFIG_PARAMS` and configuration >> +`trace2.configparams` can be used to output config values which you care >> +about(see linkgit:git-config[1). For example: > > I didn't notice this before, but this is an addition to a long section > where the examples are ------- delimited, starting with "in this > example.." usually. > > So this "print configs" seems like on odd continuation. Shouldn't this > copy the template of "Thread Events::" above. I.e. something like (I > have not tried to asciidoc render this): > > Config (def param) Events:: > We can optionally emit configuration events, see > `trace2.configParams` in linkgit:git-config[1] for how to enable > it. > + > < your example below would follow this> > > I.e. re my earlier mention of git-config we it explains > GIT_TRACE2_CONFIG_PARAMS, so perhaps it suffices to just link to > linkgit:git-config[1] for that. > > Also a nit: trace2.configParams, not trace2.configparams. These may be worth fixing, regardless of the "adding scope in the example needs to wait until 2/2" I pointed out separately. Thanks.
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > I didn't notice this before, but this is an addition to a long section > where the examples are ------- delimited, starting with "in this > example.." usually. A little puzzled. About "------- delimited", do you mean the "block" syntax? > So this "print configs" seems like on odd continuation. Shouldn't this > copy the template of "Thread Events::" above. I.e. something like (I > have not tried to asciidoc render this): I think "Events" in "Thread Events" means the thread start and end events, maybe not a template here. So I think it might be better to keep this namingi if I'm right, but I will use this candidate naming to show diff in the end. I'm not good at naming so I'm glad to accept the suggestion about it. > Config (def param) Events:: > We can optionally emit configuration events, see > `trace2.configParams` in linkgit:git-config[1] for how to enable > it. > + > < your example below would follow this> I refer to the previous sections, it's like the template is : <title> <brief one-line description> <detailed multi-lines description> So, how about this like: diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt index 77a150b30e..38d0878d85 100644 --- a/Documentation/technical/api-trace2.txt +++ b/Documentation/technical/api-trace2.txt @@ -1207,6 +1207,37 @@ at offset 508. This example also shows that thread names are assigned in a racy manner as each thread starts and allocates TLS storage. +Config (def param) Events:: + + Dump "interesting" config values to trace2 log. ++ +We can optionally emit configuration events, see +`trace2.configparams` in linkgit:git-config[1] for how to enable +it. ++ +---------------- +$ git config color.ui auto +---------------- ++ +Then, mark the config `color.ui` as "interesting" config with +`GIT_TRACE2_CONFIG_PARAMS`: ++ +---------------- +$ export GIT_TRACE2_PERF_BRIEF=1 +$ export GIT_TRACE2_PERF=~/log.perf +$ export GIT_TRACE2_CONFIG_PARAMS=color.ui +$ git version +... +$ cat ~/log.perf +d0 | main | version | | | | | ... +d0 | main | start | | 0.001642 | | | /usr/local/bin/git version +d0 | main | cmd_name | | | | | version (version) +d0 | main | def_param | | | | | color.ui:auto +d0 | main | data | r0 | 0.002100 | 0.002100 | fsync | fsync/writeout-only:0 +d0 | main | data | r0 | 0.002126 | 0.002126 | fsync | fsync/hardware-flush:0 +d0 | main | exit | | 0.002142 | | | code:0 +d0 | main | atexit | | 0.002161 | | | code:0 +---------------- == Future Work === Relationship to the Existing Trace Api (api-trace.txt) Thanks.
"tenglong.tl" <dyroneteng@gmail.com> writes: > Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes: > >> I didn't notice this before, but this is an addition to a long section > ... > I refer to the previous sections, it's like the template is : > > <title> > > <brief one-line description> > > <detailed multi-lines description> > > So, how about this like: > > diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt > index 77a150b30e..38d0878d85 100644 > --- a/Documentation/technical/api-trace2.txt > +++ b/Documentation/technical/api-trace2.txt > @@ -1207,6 +1207,37 @@ at offset 508. So, what is the outcome from this discussion? It seems that this subthread on [1/2] is blocking the two-patch series? Thanks.
Junio C Hamano <gitster@pobox.com> writes: > So, what is the outcome from this discussion? It seems that this > subthread on [1/2] is blocking the two-patch series? I think it's blocking. I replied a comment to comfirm that I understand Ævar Arnfjörð Bjarmason's suggestion and also there is an improvement change in it: https://public-inbox.org/git/20220801122515.23146-1-tenglong.tl@alibaba-inc.com/ Thanks.
diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt index 77a150b30e..ddc0bfb9c9 100644 --- a/Documentation/technical/api-trace2.txt +++ b/Documentation/technical/api-trace2.txt @@ -717,6 +717,7 @@ The "exec_id" field is a command-unique id and is only useful if the { "event":"def_param", ... + "scope":"global", "param":"core.abbrev", "value":"7" } @@ -1207,6 +1208,37 @@ at offset 508. This example also shows that thread names are assigned in a racy manner as each thread starts and allocates TLS storage. +Print Configs:: + + Dump "interesting" config values to trace2 log. ++ +The environment variable `GIT_TRACE2_CONFIG_PARAMS` and configuration +`trace2.configparams` can be used to output config values which you care +about(see linkgit:git-config[1). For example: ++ +---------------- +$ git config color.ui auto +---------------- ++ +Then, mark the config `color.ui` as "interesting" config with +`GIT_TRACE2_CONFIG_PARAMS`: ++ +---------------- +$ export GIT_TRACE2_PERF_BRIEF=1 +$ export GIT_TRACE2_PERF=~/log.perf +$ export GIT_TRACE2_CONFIG_PARAMS=color.ui +$ git version +... +$ cat ~/log.perf +d0 | main | version | | | | | ... +d0 | main | start | | 0.001642 | | | /usr/local/bin/git version +d0 | main | cmd_name | | | | | version (version) +d0 | main | def_param | | | | | color.ui:auto +d0 | main | data | r0 | 0.002100 | 0.002100 | fsync | fsync/writeout-only:0 +d0 | main | data | r0 | 0.002126 | 0.002126 | fsync | fsync/hardware-flush:0 +d0 | main | exit | | 0.002142 | | | code:0 +d0 | main | atexit | | 0.002161 | | | code:0 +---------------- == Future Work === Relationship to the Existing Trace Api (api-trace.txt)