From: Taylor Blau <me@ttaylorr.com>
To: git@vger.kernel.org
Cc: Jeff King <peff@peff.net>, Patrick Steinhardt <ps@pks.im>,
Junio C Hamano <gitster@pobox.com>
Subject: [PATCH 2/6] pack-bitmap-write.c: move commit_positions into commit_pos fields
Date: Tue, 14 May 2024 15:56:53 -0400 [thread overview]
Message-ID: <cbedff02ed1409c2b5f97596fe0b43d0a4a4d0ff.1715716605.git.me@ttaylorr.com> (raw)
In-Reply-To: <cover.1715716605.git.me@ttaylorr.com>
In 7cc8f971085 (pack-objects: implement bitmap writing, 2013-12-21), the
bitmapped_commit struct was introduced, including the 'commit_pos'
field, which has been unused ever since its introduction more than a
decade ago.
Instead, we have used the nearby `commit_positions` array leaving the
bitmapped_commit struct with an unused 4-byte field.
We could drop the `commit_pos` field as unused, and continue to store
the values in the auxiliary array. But we could also drop the array and
store the data for each bitmapped_commit struct inside of the structure
itself, which is what this patch does.
In any spot that we previously read `commit_positions[i]`, we can now
instead read `writer.selected[i].commit_pos`. There are a few spots that
need changing as a result:
- write_selected_commits_v1() is a simple transformation, since we're
just reading the field. As a result, the function no longer needs an
explicit argument to pass the commit_positions array.
- write_lookup_table() also no longer needs the explicit
commit_positions array passed in as an argument. But it still needs
to sort an array of indices into the writer.selected array to read
them in commit_pos order, so table_cmp() is adjusted accordingly.
- bitmap_writer_finish() no longer needs to allocate, populate, and
free the commit_positions table. Instead, we can just write the data
directly into each struct bitmapped_commit.
Signed-off-by: Taylor Blau <me@ttaylorr.com>
---
pack-bitmap-write.c | 41 ++++++++++++++++-------------------------
1 file changed, 16 insertions(+), 25 deletions(-)
diff --git a/pack-bitmap-write.c b/pack-bitmap-write.c
index c6c8f94cc5..2ae82b8696 100644
--- a/pack-bitmap-write.c
+++ b/pack-bitmap-write.c
@@ -670,9 +670,7 @@ static const struct object_id *oid_access(size_t pos, const void *table)
return &index[pos]->oid;
}
-static void write_selected_commits_v1(struct hashfile *f,
- uint32_t *commit_positions,
- off_t *offsets)
+static void write_selected_commits_v1(struct hashfile *f, off_t *offsets)
{
int i;
@@ -682,7 +680,7 @@ static void write_selected_commits_v1(struct hashfile *f,
if (offsets)
offsets[i] = hashfile_total(f);
- hashwrite_be32(f, commit_positions[i]);
+ hashwrite_be32(f, stored->commit_pos);
hashwrite_u8(f, stored->xor_offset);
hashwrite_u8(f, stored->flags);
@@ -690,23 +688,20 @@ static void write_selected_commits_v1(struct hashfile *f,
}
}
-static int table_cmp(const void *_va, const void *_vb, void *_data)
+static int table_cmp(const void *_va, const void *_vb)
{
- uint32_t *commit_positions = _data;
- uint32_t a = commit_positions[*(uint32_t *)_va];
- uint32_t b = commit_positions[*(uint32_t *)_vb];
+ struct bitmapped_commit *a = &writer.selected[*(uint32_t *)_va];
+ struct bitmapped_commit *b = &writer.selected[*(uint32_t *)_vb];
- if (a > b)
+ if (a->commit_pos < b->commit_pos)
+ return -1;
+ else if (a->commit_pos > b->commit_pos)
return 1;
- else if (a < b)
- return -1;
return 0;
}
-static void write_lookup_table(struct hashfile *f,
- uint32_t *commit_positions,
- off_t *offsets)
+static void write_lookup_table(struct hashfile *f, off_t *offsets)
{
uint32_t i;
uint32_t *table, *table_inv;
@@ -722,7 +717,7 @@ static void write_lookup_table(struct hashfile *f,
* bitmap corresponds to j'th bitmapped commit (among the selected
* commits) in lex order of OIDs.
*/
- QSORT_S(table, writer.selected_nr, table_cmp, commit_positions);
+ QSORT(table, writer.selected_nr, table_cmp);
/* table_inv helps us discover that relationship (i'th bitmap
* to j'th commit by j = table_inv[i])
@@ -753,7 +748,7 @@ static void write_lookup_table(struct hashfile *f,
xor_row = 0xffffffff;
}
- hashwrite_be32(f, commit_positions[table[i]]);
+ hashwrite_be32(f, writer.selected[table[i]].commit_pos);
hashwrite_be64(f, (uint64_t)offsets[table[i]]);
hashwrite_be32(f, xor_row);
}
@@ -789,7 +784,6 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
static uint16_t flags = BITMAP_OPT_FULL_DAG;
struct strbuf tmp_file = STRBUF_INIT;
struct hashfile *f;
- uint32_t *commit_positions = NULL;
off_t *offsets = NULL;
uint32_t i;
@@ -814,22 +808,20 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
if (options & BITMAP_OPT_LOOKUP_TABLE)
CALLOC_ARRAY(offsets, index_nr);
- ALLOC_ARRAY(commit_positions, writer.selected_nr);
-
for (i = 0; i < writer.selected_nr; i++) {
struct bitmapped_commit *stored = &writer.selected[i];
- int commit_pos = oid_pos(&stored->commit->object.oid, index, index_nr, oid_access);
+ int commit_pos = oid_pos(&stored->commit->object.oid, index,
+ index_nr, oid_access);
if (commit_pos < 0)
BUG(_("trying to write commit not in index"));
-
- commit_positions[i] = commit_pos;
+ stored->commit_pos = commit_pos;
}
- write_selected_commits_v1(f, commit_positions, offsets);
+ write_selected_commits_v1(f, offsets);
if (options & BITMAP_OPT_LOOKUP_TABLE)
- write_lookup_table(f, commit_positions, offsets);
+ write_lookup_table(f, offsets);
if (options & BITMAP_OPT_HASH_CACHE)
write_hash_cache(f, index, index_nr);
@@ -844,6 +836,5 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
die_errno("unable to rename temporary bitmap file to '%s'", filename);
strbuf_release(&tmp_file);
- free(commit_positions);
free(offsets);
}
--
2.45.1.151.g7cc3499008c
next prev parent reply other threads:[~2024-05-14 19:56 UTC|newest]
Thread overview: 14+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-14 19:56 [PATCH 0/6] pack-bitmap: various pack-bitmap-write cleanups Taylor Blau
2024-05-14 19:56 ` [PATCH 1/6] object.h: add flags allocated by pack-bitmap.h Taylor Blau
2024-05-14 19:56 ` Taylor Blau [this message]
2024-05-14 19:56 ` [PATCH 3/6] pack-bitmap: avoid use of static `bitmap_writer` Taylor Blau
2024-05-14 19:57 ` [PATCH 4/6] pack-bitmap: drop unused `max_bitmaps` parameter Taylor Blau
2024-05-14 19:57 ` [PATCH 5/6] pack-bitmap-write.c: avoid uninitialized 'write_as' field Taylor Blau
2024-05-15 9:05 ` Patrick Steinhardt
2024-05-15 13:29 ` Taylor Blau
2024-05-14 19:57 ` [PATCH 6/6] pack-bitmap: introduce `bitmap_writer_free()` Taylor Blau
2024-05-15 9:05 ` Patrick Steinhardt
2024-05-15 15:58 ` Taylor Blau
2024-05-15 6:18 ` [PATCH 0/6] pack-bitmap: various pack-bitmap-write cleanups Jeff King
2024-05-15 9:05 ` Patrick Steinhardt
2024-05-15 15:58 ` Taylor Blau
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=cbedff02ed1409c2b5f97596fe0b43d0a4a4d0ff.1715716605.git.me@ttaylorr.com \
--to=me@ttaylorr.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=peff@peff.net \
--cc=ps@pks.im \
/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 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).