All of lore.kernel.org
 help / color / mirror / 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 v2 4/5] bitmap: add trace2 outputs during open "bitmap" file
Date: Fri,  6 May 2022 20:43:33 +0800	[thread overview]
Message-ID: <20220506124333.58956-1-dyroneteng@gmail.com> (raw)
In-Reply-To: <8005642d-979f-8e48-7a93-07b8a888bdc7@jeffhostetler.com>

> This might just be a style thing, but rather than logging the pathname
> in a separate data_string message, you can use the _printf version of
> region_enter and region_leave to also print the name of the
> path -- like I did in read-cache.c for the "do_read_index"
> calls.
> 
> ... | region_enter | ... | index | label:do_read_index .git/index
> ...
> ... | region_leave | ... | index | label:do_read_index .git/index

Appreciate for the input about the _printf version, we can choose to
let the region_enter and region_leave to print the pathname by moving
the related "midx_bitmap_filename()" and "pack_bitmap_filename" at
front, but it's not enough because both midx and normal bitmap support
multiple opening, so it's likely we keep on the current way using
"trace2_data_string()" in "open_pack_bitmap_1()" and "open_midx_bitmap_1()"
is a simpler solution.

I'm not sure If I totally get the meaning about your suggestion,
so correct me if I understand you wrong.

> As AEvar suggests in another message in this thread, I'm not sure if
> you need the region timing here for reading the bitmap, but all of
> the error and any other data messages will be bounded between the
> region_enter and region_leave events and that might (or might not)
> be helpful.

I think it's needed in my opinion, the bounded between the region is
helpful, especially when we want to know the detailed debug info like
we do in "open_midx_bitmap_1()".  


> Also, I agree with AEvar's statements about using error() and getting
> the trace2 error messages for free and not needing some of the
> trace2_data_string() messages that you have later in this file.
> 
> I wonder if it would be worth adding the pathname of the invalid
> file to those new error messages.  Granted you'll have it in the
> trace2 log, but then you'll also get it on stderr if that would
> be helpful.

I think I will remove the redundant "trace2_data_string()" code when
it will return by "error()".


Thanks.

  reply	other threads:[~2022-05-06 12:43 UTC|newest]

Thread overview: 129+ 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 [this message]
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     ` Teng Long
2022-06-12  7:44       ` [PATCH v3 0/5] trace2 output for bitmap decision path 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
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

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

  git send-email \
    --in-reply-to=20220506124333.58956-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.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.