All of lore.kernel.org
 help / color / mirror / Atom feed
From: Teng Long <dyroneteng@gmail.com>
To: dyroneteng@gmail.com
Cc: avarab@gmail.com, derrickstolee@github.com, git@vger.kernel.org,
	gitster@pobox.com, me@ttaylorr.com, peff@peff.net,
	tenglong.tl@alibaba-inc.com
Subject: [PATCH v3 0/2] pack-bitmap.c: avoid exposing absolute paths
Date: Thu, 10 Nov 2022 15:10:10 +0800	[thread overview]
Message-ID: <cover.1668063122.git.dyroneteng@gmail.com> (raw)
In-Reply-To: <cover.1667470481.git.dyroneteng@gmail.com>

From: Teng Long <dyroneteng@gmail.com>

Diff since v2:

* remove unnecessary comments.
* use "GIT_TRACE2_EVENT" instead of "GIT_TRACE_PERF".
* improve commit message of [1/2].

Thanks.

Teng Long (2):
  pack-bitmap.c: remove unnecessary "open_pack_index()" calls
  pack-bitmap.c: avoid exposing absolute paths

 pack-bitmap.c           | 13 ++++++-------
 t/t5310-pack-bitmaps.sh |  5 +++--
 2 files changed, 9 insertions(+), 9 deletions(-)

Range-diff against v2:
2:  7ac9b859f3 ! 1:  aaeb021538 pack-bitmap.c: remove unnecessary "open_pack_index()" calls
    @@ Metadata
      ## Commit message ##
         pack-bitmap.c: remove unnecessary "open_pack_index()" calls
     
    -    Everytime when calling "open_pack_bitmap_1()", we will firstly call
    -    "open_pack_index(packfile)" to check the index, then further check
    -    again in "is_pack_valid()" before mmap the bitmap file. So, let's
    -    remove the first check here.
    +    When trying to open a pack bitmap, we call open_pack_bitmap_1() in a
    +    loop, during which it tries to open up the pack index corresponding
    +    with each available pack.
     
    -    The relate discussion:
    -    https://public-inbox.org/git/Y2IiSU1L+bJPUioV@coredump.intra.peff.net/#t
    +    It's likely that we'll end up relying on objects in that pack later
    +    in the process (in which case we're doing the work of opening the
    +    pack index optimistically), but not guaranteed.
    +
    +    For instance, consider a repository with a large number of small
    +    packs, and one large pack with a bitmap. If we see that bitmap pack
    +    last in our loop which calls open_pack_bitmap_1(), the current code
    +    will have opened *all* pack index files in the repository. If the
    +    request can be served out of the bitmapped pack alone, then the time
    +    spent opening these idx files was wasted.S
    +
    +    Since open_pack_bitmap_1() calls is_pack_valid() later on (which in
    +    turns calls open_pack_index() itself), we can just drop the earlier
    +    call altogether.
     
         Signed-off-by: Teng Long <dyroneteng@gmail.com>
     
1:  de941f58f9 ! 2:  9d5a491887 pack-bitmap.c: avoid exposing absolute paths
    @@ pack-bitmap.c: static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
      		get_midx_filename(&buf, midx->object_dir);
     -		/* ignore extra bitmap file; we can only handle one */
     -		warning(_("ignoring extra bitmap file: '%s'"), buf.buf);
    -+		/* ignore extra midx bitmap files; we can only handle one */
     +		trace2_data_string("bitmap", the_repository,
     +				   "ignoring extra midx bitmap file", buf.buf);
      		close(fd);
    @@ pack-bitmap.c: static int open_pack_bitmap_1(struct bitmap_index *bitmap_git, st
      	if (bitmap_git->pack || bitmap_git->midx) {
     -		/* ignore extra bitmap file; we can only handle one */
     -		warning(_("ignoring extra bitmap file: '%s'"), packfile->pack_name);
    -+		/* ignore extra bitmap files; we can only handle one */
     +		trace2_data_string("bitmap", the_repository,
     +				   "ignoring extra bitmap file", packfile->pack_name);
      		close(fd);
    @@ t/t5310-pack-bitmaps.sh: test_bitmap_cases () {
      
     -			git rev-list --use-bitmap-index HEAD 2>err &&
     -			grep "ignoring extra bitmap file" err
    -+			GIT_TRACE2_PERF=$(pwd)/trace2.txt git rev-list --use-bitmap-index HEAD &&
    ++			GIT_TRACE2_EVENT=$(pwd)/trace2.txt git rev-list --use-bitmap-index HEAD &&
     +			grep "opened bitmap" trace2.txt &&
     +			grep "ignoring extra bitmap" trace2.txt
      		)
-- 
2.38.1.383.g7ac9b859f31.dirty


  parent reply	other threads:[~2022-11-10  7:10 UTC|newest]

Thread overview: 67+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-08-26  7:09 [PATCH 0/1] pack-bitmap.c: avoid exposing absolute paths Teng Long
2022-08-26  7:09 ` [PATCH 1/1] " Teng Long
2022-08-26 16:34 ` [PATCH 0/1] " Junio C Hamano
2022-08-29  2:48   ` Teng Long
2022-10-26 21:42     ` Taylor Blau
2022-10-26 23:19       ` Ævar Arnfjörð Bjarmason
2022-10-31 13:20         ` Teng Long
2022-10-27 20:45       ` Jeff King
2022-10-30 18:42         ` Taylor Blau
2022-10-31 12:22           ` [PATCH 0/1] pack-bitmap.c: avoid exposing absolute paths Taylor Blau <me@ttaylorr.com> writes: Teng Long
2022-11-02  5:37         ` [PATCH 0/1] pack-bitmap.c: avoid exposing absolute paths Teng Long
2022-11-02  7:54           ` Jeff King
2022-11-02 13:52             ` Teng Long
2022-10-31 13:13       ` Teng Long
2022-11-03  1:00         ` Taylor Blau
2022-11-02  9:20 ` Ævar Arnfjörð Bjarmason
2022-11-02 13:04   ` Teng Long
2022-11-02 12:56 ` [PATCH v2 " Teng Long
2022-11-02 12:56   ` [PATCH v2 1/1] " Teng Long
2022-11-03  1:16     ` Taylor Blau
2022-11-03  9:35       ` Teng Long
2022-11-05  0:35         ` Taylor Blau
2022-11-03  1:21   ` [PATCH v2 0/1] " Taylor Blau
2022-11-03  8:42     ` Teng Long
2022-11-04  3:17   ` [PATCH v3 0/2] " Teng Long
2022-11-04  3:17     ` [PATCH v3 1/2] " Teng Long
2022-11-04 22:11       ` Taylor Blau
2022-11-04  3:17     ` [PATCH v3 2/2] pack-bitmap.c: remove unnecessary "open_pack_index()" calls Teng Long
2022-11-04 22:09       ` Taylor Blau
2022-11-04 22:13     ` [PATCH v3 0/2] pack-bitmap.c: avoid exposing absolute paths Taylor Blau
2022-11-10  7:10     ` Teng Long [this message]
2022-11-10  7:10       ` [PATCH v3 1/2] pack-bitmap.c: remove unnecessary "open_pack_index()" calls Teng Long
2022-11-14 22:03         ` Jeff King
2022-11-14 22:14           ` Taylor Blau
2022-11-14 22:31             ` Jeff King
2022-11-14 22:50               ` Taylor Blau
2022-11-10  7:10       ` [PATCH v3 2/2] pack-bitmap.c: avoid exposing absolute paths Teng Long
2022-11-11 22:26       ` [PATCH v3 0/2] " Taylor Blau
2022-11-14 22:23         ` Jeff King
2022-11-17 14:19           ` Teng Long
2022-11-17 15:03             ` Jeff King
2022-11-17 21:57               ` Taylor Blau
2022-11-21  3:27                 ` Teng Long
2022-11-21 12:16     ` [PATCH v4 0/4] " Teng Long
2022-11-21 12:16       ` [PATCH v4 1/4] pack-bitmap.c: remove unnecessary "open_pack_index()" calls Teng Long
2022-11-21 12:16       ` [PATCH v4 2/4] pack-bitmap.c: avoid exposing absolute paths Teng Long
2022-11-21 12:16       ` [PATCH v4 3/4] pack-bitmap.c: break out of the bitmap loop early if not tracing Teng Long
2022-11-21 23:27         ` Junio C Hamano
2022-11-28 13:09           ` Teng Long
2022-11-21 12:16       ` [PATCH v4 4/4] pack-bitmap.c: trace bitmap ignore logs when midx-bitmap is found Teng Long
2022-11-21 19:09         ` Jeff King
2022-11-21 23:29           ` Junio C Hamano
2022-11-28 12:29             ` Teng Long
2022-11-28 12:37           ` Teng Long
2022-11-29  1:27             ` Jeff King
2022-11-29 13:14               ` Teng Long
2022-11-21 19:04       ` [PATCH v4 0/4] pack-bitmap.c: avoid exposing absolute paths Jeff King
2022-11-28 12:48         ` Teng Long
2022-11-28 14:09       ` [PATCH v5 " Teng Long
2022-11-28 14:09         ` [PATCH v5 1/4] pack-bitmap.c: remove unnecessary "open_pack_index()" calls Teng Long
2022-11-28 14:09         ` [PATCH v5 2/4] pack-bitmap.c: avoid exposing absolute paths Teng Long
2022-11-28 14:09         ` [PATCH v5 3/4] pack-bitmap.c: break out of the bitmap loop early if not tracing Teng Long
2022-11-28 23:26           ` Taylor Blau
2022-11-29 13:17             ` Teng Long
2022-11-28 14:09         ` [PATCH v5 4/4] pack-bitmap.c: trace bitmap ignore logs when midx-bitmap is found Teng Long
2022-11-28 23:30         ` [PATCH v5 0/4] pack-bitmap.c: avoid exposing absolute paths Taylor Blau
2022-11-29 13:21           ` Teng Long

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=cover.1668063122.git.dyroneteng@gmail.com \
    --to=dyroneteng@gmail.com \
    --cc=avarab@gmail.com \
    --cc=derrickstolee@github.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=me@ttaylorr.com \
    --cc=peff@peff.net \
    --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.