Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> writes: > On Fri, May 04 2018, Jakub Narebski wrote: > > (Just off-the cuff here and I'm surely about to be corrected by > Derrick...) > >> * What to do about merge commits, and octopus merges in particular? >> Should Bloom filter be stored for each of the parents? How to ensure >> fast access then (fixed-width records) - use large edge list? > > You could still store it fixed with, you'd just say that if you > encounter a merge with N parents the filter wouldn't store files changed > in that commit, but rather whether any of the N (including the merge) > had changes to files as of the their common merge-base. Well, that is one solution: to store union of changes (sum of changes) from all parents of a merge commit in a Bloom filter for a merge. But this wouldn't help us to skip merge entirely, because which parent we would walk then? But perhaps I am mistaken, and that does not matter. > Then if they did you'd need to walk all sides of the merge where each > commit would also have the filter to figure out where the change(s) > was/were, but if they didn't you could skip straight to the merge base > and keep walking. Another solution that I thought of is to use the same mechanism that commit-graph file uses for storing merges: store Bloom filters for first two parents, and if there are more parents (octopus merge), store Bloom filters for the remaining parents in large edge extension for Bloom filters. This meant accepting some padding waste for CDAT chunk, to have faster access. We could do the same for Bloom filters, but it may mean quite a bit of waste, depending on how many bits Bloom filter would use... but there is another solution: for merge commits store Bloom filters for first two parents that are half the size - this means of course more false positives, but it may be acceptable solution. > Which, on the topic of what else a commit graph could store: A mapping > from merge commits of N parents to the merge-base of those commits. The problem is that those N parents may have more than one merge-base, and if so then those merge-bases may have also multiple merge-bases, recursively (what 'recursive' merge strategy handles). Though this could be solved with 'large merge-base list' extension, just like existing EDGE chunk - I think we can assume that most merge parents have only single merge-base (but I have not checked this). > You could also store nothing for merges (or only files the merge itself > changed v.s. its parents). Derrick talked about how the bloom filter > implementation has a value that's "Didn't compute (for whatever reason), > look at it manually". Right, another solution could be to store nothing for merges, or store Bloom filter for changes against only first parent. The goal of Bloom filter is to avoid calculating diff if we don't need to. Derrick, could you tell us what solution VSTS uses for Bloom filters on merge commits? Thanks in advance. >> * Then there is problem of rename and copying detection - I think we can >> simply ignore it: unless someone has an idea about how to handle it? >> >> Though this means that "git log --follow <file>" wouldn't get any >> speedup, and neither the half of "git gui blame" that runs "git blame >> --incremental -C -C -w" -- the one that allows code copying and >> movement detection. > > Couldn't the bloom filter also speed up --follow if you did two passes > through the history? The first to figure out all files that ever changed > names, and then say you did `--follow sha1-name.c` on git.git. The > filter would have had all the bits for both sha1_name.c and sha1-name.c > set on all commits that touched either for all of the history. > > Of course this would only work with a given default value of -M<n>, but > on the assumption that most users left it at the default, and > furthermore that renames weren't so common as to make the filter useless > with too many false-positives as a result, it might be worth it. If you I think it would be much simpler to just ensure that we store in Bloom filter as changed files also pure renames, and leave doing rename detection to the walk. This way we do not fix old rename detecion algorithm in stone. The walk would simply change the name of file it would ask Bloom filters about. Thank you for your comments, -- Jakub Narębski
