On 6/26/2022 9:10 AM, Abhradeep Chakraborty via GitGitGadget wrote:
> From: Abhradeep Chakraborty <chakrabortyabhradeep79@xxxxxxxxx>
>
> Teach git to provide a way for users to enable/disable bitmap lookup
> table extension by providing a config option named 'writeBitmapLookupTable'.
> Default is true.
I wonder if it makes sense to have it default to 'false' for now, but to
change that default after the feature has been shipped and running in
production for a while.
> Also add test to verify writting of lookup table.
s/writting/writing/
> +pack.writeBitmapLookupTable::
> + When true, git will include a "lookup table" section in the
I think you should either use "Git" when talking about the software
generally, OR use "`git repack --write-bitmap-index` will include..."
> + bitmap index (if one is written). This table is used to defer
> + loading individual bitmaps as late as possible. This can be
> + beneficial in repositories which have relatively large bitmap
s/which/that/
(I'm pretty sure that "that" is better. We're trying to restrict the set
of repositories we are talking about, not implying that all repositories
have this property.)
> + indexes. Defaults to true.
> +
> --- a/pack-bitmap-write.c
> +++ b/pack-bitmap-write.c
> @@ -713,6 +713,7 @@ static void write_lookup_table(struct hashfile *f,
> for (i = 0; i < writer.selected_nr; i++)
> table_inv[table[i]] = i;
>
> + trace2_region_enter("pack-bitmap-write", "writing_lookup_table", the_repository);
> for (i = 0; i < writer.selected_nr; i++) {
> struct bitmapped_commit *selected = &writer.selected[table[i]];
> uint32_t xor_offset = selected->xor_offset;
> @@ -725,6 +726,7 @@ static void write_lookup_table(struct hashfile *f,
>
> free(table);
> free(table_inv);
> + trace2_region_leave("pack-bitmap-write", "writing_lookup_table", the_repository);
> }
These lines seem misplaced. Maybe they were meant for the previous
patch?
Thanks,
-Stolee
