pack-bitmap-write.c: move commit_positions into commit_pos fields

In 7cc8f97108 (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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
maint
Taylor Blau 2024-05-14 15:56:53 -04:00 committed by Junio C Hamano
parent b174a97a54
commit 94830fcacc
1 changed files with 16 additions and 25 deletions

View File

@ -670,9 +670,7 @@ static const struct object_id *oid_access(size_t pos, const void *table)
return &index[pos]->oid; return &index[pos]->oid;
} }


static void write_selected_commits_v1(struct hashfile *f, static void write_selected_commits_v1(struct hashfile *f, off_t *offsets)
uint32_t *commit_positions,
off_t *offsets)
{ {
int i; int i;


@ -682,7 +680,7 @@ static void write_selected_commits_v1(struct hashfile *f,
if (offsets) if (offsets)
offsets[i] = hashfile_total(f); 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->xor_offset);
hashwrite_u8(f, stored->flags); 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; struct bitmapped_commit *a = &writer.selected[*(uint32_t *)_va];
uint32_t a = commit_positions[*(uint32_t *)_va]; struct bitmapped_commit *b = &writer.selected[*(uint32_t *)_vb];
uint32_t b = commit_positions[*(uint32_t *)_vb];


if (a > b) if (a->commit_pos < b->commit_pos)
return 1;
else if (a < b)
return -1; return -1;
else if (a->commit_pos > b->commit_pos)
return 1;


return 0; return 0;
} }


static void write_lookup_table(struct hashfile *f, static void write_lookup_table(struct hashfile *f, off_t *offsets)
uint32_t *commit_positions,
off_t *offsets)
{ {
uint32_t i; uint32_t i;
uint32_t *table, *table_inv; 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 * bitmap corresponds to j'th bitmapped commit (among the selected
* commits) in lex order of OIDs. * 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 /* table_inv helps us discover that relationship (i'th bitmap
* to j'th commit by j = table_inv[i]) * to j'th commit by j = table_inv[i])
@ -753,7 +748,7 @@ static void write_lookup_table(struct hashfile *f,
xor_row = 0xffffffff; 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_be64(f, (uint64_t)offsets[table[i]]);
hashwrite_be32(f, xor_row); 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; static uint16_t flags = BITMAP_OPT_FULL_DAG;
struct strbuf tmp_file = STRBUF_INIT; struct strbuf tmp_file = STRBUF_INIT;
struct hashfile *f; struct hashfile *f;
uint32_t *commit_positions = NULL;
off_t *offsets = NULL; off_t *offsets = NULL;
uint32_t i; uint32_t i;


@ -814,22 +808,20 @@ void bitmap_writer_finish(struct pack_idx_entry **index,
if (options & BITMAP_OPT_LOOKUP_TABLE) if (options & BITMAP_OPT_LOOKUP_TABLE)
CALLOC_ARRAY(offsets, index_nr); CALLOC_ARRAY(offsets, index_nr);


ALLOC_ARRAY(commit_positions, writer.selected_nr);

for (i = 0; i < writer.selected_nr; i++) { for (i = 0; i < writer.selected_nr; i++) {
struct bitmapped_commit *stored = &writer.selected[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) if (commit_pos < 0)
BUG(_("trying to write commit not in index")); BUG(_("trying to write commit not in index"));

stored->commit_pos = commit_pos;
commit_positions[i] = commit_pos;
} }


write_selected_commits_v1(f, commit_positions, offsets); write_selected_commits_v1(f, offsets);


if (options & BITMAP_OPT_LOOKUP_TABLE) if (options & BITMAP_OPT_LOOKUP_TABLE)
write_lookup_table(f, commit_positions, offsets); write_lookup_table(f, offsets);


if (options & BITMAP_OPT_HASH_CACHE) if (options & BITMAP_OPT_HASH_CACHE)
write_hash_cache(f, index, index_nr); 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); die_errno("unable to rename temporary bitmap file to '%s'", filename);


strbuf_release(&tmp_file); strbuf_release(&tmp_file);
free(commit_positions);
free(offsets); free(offsets);
} }