Message ID | pull.1266.v3.git.1656924376.gitgitgadget@gmail.com (mailing list archive) |
---|---|
Headers | show |
Series | bitmap: integrate a lookup table extension to the bitmap format | expand |
Oops, I forgot to edit the PR cover letter! I am adding it here. Sorry for that :P Changes since v2: * Log messages related issues are fixed. * `pack.writeBitmapLookupTable` is now by default disabled. * Documentations are improved. * `xor_row` is used instead of `xor_pos` in triplets. * In `pack-bitmap-write.c`, `off_t *` is used for `offsets` array (Instead of `uint64_t *`). * `struct bitmap_lookup_table_triplet` is introduced and functions Like `triplet_get_offset()` and `triplet_get_xor_pos()` are removed. * `table_size` is getting subtracted from `index_end` irrespective of the value of `GIT_TEST_READ_COMMIT_TABLE`. * xor stack filling loop will stop iterating if a xor bitmap is already stored/parsed. * The stack will now store `bitmap_lookup_table_xor_item` items Of plain xor_row. * bitmap related test files are reformatted to allow repeating of tests with bitmap extension enabled. * comments are added. Thanks :)
"Abhradeep Chakraborty via GitGitGadget" <gitgitgadget@gmail.com> writes: > When parsing the .bitmap file, git loads all the bitmaps one by one even if > some of the bitmaps are not necessary. We can remove this overhead by > loading only the necessary bitmaps. A look up table extension can solve this > issue. > > Changes since v1: > > This is the second version which addressed all (I think) the reviews. Please > notify me if some reviews are not addressed :) Is this the second version that is labeled as "v3" ;-)? > Documentation/technical/bitmap-format.txt | 39 ++ I haven't tried merging it yet, but doesn't [1/6] overlap with and semantically depend on your other series that touch the formatting of this file? Thanks.
Junio C Hamano <gitster@pobox.com> wrote: > > This is the second version which addressed all (I think) the reviews. Please > > notify me if some reviews are not addressed :) > > Is this the second version that is labeled as "v3" ;-)? Hi junio, No, it is actually the third version. I forgot to update the cover letter :P. I am using Github's gitgitgadget to submit PRs and it uses PR description as the cover letter. So before submitting a new version of patchset, PR description must be updated which I missed this time. I wrote a reply comment[1] where you can find a summary of all the new changes. [1] https://lore.kernel.org/git/20220704163506.76162-1-chakrabortyabhradeep79@gmail.com/ > > Documentation/technical/bitmap-format.txt | 39 ++ > > I haven't tried merging it yet, but doesn't [1/6] overlap with and > semantically depend on your other series that touch the formatting > of this file? Correct, [1/6] indeed depends on my previous patch series[2] and it is assuming that that series has already been merged. As far as it seems, it will not create any merge conflicts while merging but I am not sure. This would be interesting to see. [2] https://lore.kernel.org/git/pull.1246.v4.git.1655355834.gitgitgadget@gmail.com/ Thanks :)
On 07-07-2022 14:18, Abhradeep Chakraborty wrote: > > Junio C Hamano <gitster@pobox.com> wrote: > >>> Documentation/technical/bitmap-format.txt | 39 ++ >> >> I haven't tried merging it yet, but doesn't [1/6] overlap with and >> semantically depend on your other series that touch the formatting >> of this file? > > Correct, [1/6] indeed depends on my previous patch series[2] and it > is assuming that that series has already been merged. I suppose it's the opposite. A quick check shows that the patch applies cleanly over 'master' but fails to apply over 'next' which has the changes from your other patch series. So, the base branch for [1/6] is 'master'. The other 5 patches clearly don't conflict. > As far as it seems, > it will not create any merge conflicts while merging but I am not sure. > This would be interesting to see. > Since the first hunk of 1/6 and your other series touch the same area of Documentation/technical/bitmap-format.txt, the changes conflict. Junio might be able to handle this one. If not, you would need to look into separate 1/6 and based it over your other series to avoid the conflict. -- Sivaraam
Kaartic Sivaraam <kaartic.sivaraam@gmail.com> wrote: >> Correct, [1/6] indeed depends on my previous patch series[2] and it >> is assuming that that series has already been merged. > > I suppose it's the opposite. A quick check shows that the patch applies > cleanly over 'master' but fails to apply over 'next' which has the > changes from your other patch series. So, the base branch for [1/6] > is 'master'. The other 5 patches clearly don't conflict. Actually by saying "[1/6] indeed depends on my previous patch series[2] and it is assuming that that series has already been merged.", I wanted to mean that the format followed in this patch (e.g. description list, indentation etc.) is dependent on the format changes introduced in that Patch series. If you say about the base branch, yes, you're right. The base branch is 'Master'. > Since the first hunk of 1/6 and your other series touch the same area > of Documentation/technical/bitmap-format.txt, the changes conflict. > Junio might be able to handle this one. If not, you would need to look > into separate 1/6 and based it over your other series to avoid the > conflict. Oh, I see. I have no problem doing that :) Let me know if Junio face any problem fixing the conflict. Thanks :)