| Message ID | cover.1724793201.git.me@ttaylorr.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | pack-objects: brown-paper-bag fixes for multi-pack reuse | expand |
Taylor Blau <me@ttaylorr.com> writes: > This series fixes a couple of issues (some cosmetic, others less so) in > multi-pack reuse noticed when rolling this out over a few real-world, > internal repositories on GitHub's servers. I cannot claim I got all the detail that went into steps 2 & 3 right, but I was irritated enough that the topic was left in the "Needs review" state, so I gave a look at the tail end of the series, and they were pleasant read. Thanks.
On Wed, Sep 04, 2024 at 11:56:27AM -0700, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > > This series fixes a couple of issues (some cosmetic, others less so) in > > multi-pack reuse noticed when rolling this out over a few real-world, > > internal repositories on GitHub's servers. > > I cannot claim I got all the detail that went into steps 2 & 3 > right, but I was irritated enough that the topic was left in the > "Needs review" state, so I gave a look at the tail end of the > series, and they were pleasant read. Thanks. They were anything *but* a pleasant debugging session, but I'm glad that the end result was palatable ;-). Thanks, Taylor
On Wed, Sep 04, 2024 at 11:56:27AM -0700, Junio C Hamano wrote: > Taylor Blau <me@ttaylorr.com> writes: > > > This series fixes a couple of issues (some cosmetic, others less so) in > > multi-pack reuse noticed when rolling this out over a few real-world, > > internal repositories on GitHub's servers. > > I cannot claim I got all the detail that went into steps 2 & 3 > right, but I was irritated enough that the topic was left in the > "Needs review" state, so I gave a look at the tail end of the > series, and they were pleasant read. Sorry, I'm probably the most qualified reviewer here. I read through the early patches, and I think the fix is correct, along with the preparation in patch 2. With the partial disclaimer that I helped with the early debugging, and my blind flailing at suggestions accidentally led Taylor to the right answer. I don't think that biases my perspective, but maybe. ;) -Peff
Jeff King <peff@peff.net> writes: > On Wed, Sep 04, 2024 at 11:56:27AM -0700, Junio C Hamano wrote: > >> Taylor Blau <me@ttaylorr.com> writes: >> >> > This series fixes a couple of issues (some cosmetic, others less so) in >> > multi-pack reuse noticed when rolling this out over a few real-world, >> > internal repositories on GitHub's servers. >> >> I cannot claim I got all the detail that went into steps 2 & 3 >> right, but I was irritated enough that the topic was left in the >> "Needs review" state, so I gave a look at the tail end of the >> series, and they were pleasant read. > > Sorry, I'm probably the most qualified reviewer here. I read through > the early patches, and I think the fix is correct, along with the > preparation in patch 2. > > With the partial disclaimer that I helped with the early debugging, and > my blind flailing at suggestions accidentally led Taylor to the right > answer. I don't think that biases my perspective, but maybe. ;) Thanks.
This series fixes a couple of issues (some cosmetic, others less so) in multi-pack reuse noticed when rolling this out over a few real-world, internal repositories on GitHub's servers. The patches are laid out as follows: - The first three patches demonstrate, prepare for, and fix a significant bug with multi-pack reuse which results in all sorts of strange behavior (explained in detail in the third commit of this series). - The fourth patch is a minor (mostly cosmetic) performance optimization that avoids duplicate calls to pack_pos_to_offset() when performing pack-reuse with a MIDX bitmap. - The final patch is a cosmetic fix to avoid using the value of a constant instead of the name constant itself. Thanks in advance for your review! Taylor Blau (5): t/t5332-multi-pack-reuse.sh: verify pack generation with --strict pack-bitmap: tag bitmapped packs with their corresponding MIDX builtin/pack-objects.c: translate bit positions during pack-reuse pack-bitmap.c: avoid repeated `pack_pos_to_offset()` during reuse builtin/pack-objects.c: do not open-code `MAX_PACK_OBJECT_HEADER` builtin/pack-objects.c | 46 +++++++++++++++++++++++++++++-------- midx.c | 1 + pack-bitmap.c | 12 ++++++---- pack-bitmap.h | 1 + t/t5332-multi-pack-reuse.sh | 35 ++++++++++++++++++++++++---- 5 files changed, 78 insertions(+), 17 deletions(-) base-commit: 159f2d50e75c17382c9f4eb7cbda671a6fa612d1