On Tue, Mar 26, 2024 at 02:08:35PM +0100, Dirk Gouders wrote: > The 4th round of this series. > > Chances are that I just waste your time with my attemt [4/5]. > My appologies in advance, should this be the case. > > Recently, there was a discussion [1] on the groff mailing list and I > guess I couldn't resist to try to practice what I read in the linked > resources ;-) > > [1] https://lists.gnu.org/archive/html/groff/2024-03/msg00014.html > > Could be that the remaining controversal part of [4/5] should just be > left untouched, because it is consuming so much time -- I summarized > all those versions, so that all incarnations can be compared in one > view: > > * Original: > > Asking `traverse_commit_list_filtered()` to populate the `omitted` > list means that our object walk does not perform any better than an > unfiltered object walk; all reachable objects are walked in order to > populate the list. > > * v3: > > Note that this means that our object walk will not perform any better > than an unfiltered object walk; all reachable objects are walked in > order to populate the list. > > * Junio's suggestion (with minor rearrangement): > > Note that our object walk with this function will not perform any > better than the previous unfiltered walk, because all reachable > objects need to be walked in order to populate the list of filtered > objects. > > * Kyle's suggestion: > > Note that requesting the list of filtered objects may have performance > implications; all reachable objects will be visited in order to > populate the list of filtered objects. > > * My new attempt (v4): > > This list of filtered objects may have performance implications, > however, because despite filtering objects, the possibly much larger > set of all reachable objects must be processed in order to populate > that list. I agree with the issues Junio raised on this phrasing, and trust in Junio's judgement to get to a clear phrasing :) I'll be unresponsive to email for at least the next two weeks, so please don't block awaiting my response on any future rerolls. > > -- > Changes in v4: > * Used the proper `git show` for references in [1/5] and [3/5] > * Another attempt to write clear speach in [4/5] > > Changes in v3: > * Reword the description in [4/5] > * Add a missing slash in [5/5] > > Changes in v2: > * Added Emily to Cc in the hope for a review > * Remove superfluous tags from [1/5] and [3/5] > * Replace bashism `|&` by `2>&1 |` in [5/5] > -- > Dirk Gouders (5): > MyFirstObjectWalk: use additional arg in config_fn_t > MyFirstObjectWalk: fix misspelled "builtins/" > MyFirstObjectWalk: fix filtered object walk > MyFirstObjectWalk: fix description for counting omitted objects > MyFirstObjectWalk: add stderr to pipe processing > > Documentation/MyFirstObjectWalk.txt | 37 ++++++++++++++++------------- > 1 file changed, 21 insertions(+), 16 deletions(-) > > Range-diff against v3: > 1: 0eeb4b78ac ! 1: 102cbc54c4 MyFirstObjectWalk: use additional arg in config_fn_t > @@ Metadata > ## Commit message ## > MyFirstObjectWalk: use additional arg in config_fn_t > > - Commit a4e7e317 (config: add ctx arg to config_fn_t) added a fourth > - argument to config_fn_t but did not change relevant function calls > - in Documentation/MyFirstObjectWalk.txt. > + Commit a4e7e317f8 (config: add ctx arg to config_fn_t, 2023-06-28) > + added a fourth argument to config_fn_t but did not change relevant > + function calls in Documentation/MyFirstObjectWalk.txt. > > Fix those calls and the example git_walken_config() to use > that additional argument. > 2: 3122ae2472 = 2: 5fb7953f31 MyFirstObjectWalk: fix misspelled "builtins/" > 3: f21348ab80 ! 3: b88518df0b MyFirstObjectWalk: fix filtered object walk > @@ Metadata > ## Commit message ## > MyFirstObjectWalk: fix filtered object walk > > - Commit f0d2f849 (MyFirstObjectWalk: update recommended usage) > - changed a call of parse_list_objects_filter() in a way that > - probably never worked: parse_list_objects_filter() always needed a > - pointer as its first argument. > + Commit f0d2f84919 (MyFirstObjectWalk: update recommended usage, > + 2022-03-09) changed a call of parse_list_objects_filter() in a way > + that probably never worked: parse_list_objects_filter() always needed > + a pointer as its first argument. > > Fix this by removing the CALLOC_ARRAY and passing the address of > rev->filter to parse_list_objects_filter() in accordance to > 4: cfa4b9ce50 ! 4: 11510630af MyFirstObjectWalk: fix description for counting omitted objects > @@ Documentation/MyFirstObjectWalk.txt: points to the same tree object as its grand > -reachable objects are walked in order to populate the list. > +filter, like with `git log --filter=<spec> --filter-print-omitted`. To do this, > +change `traverse_commit_list()` to `traverse_commit_list_filtered()`, which is > -+able to populate an `omitted` list. Note that this means that our object walk > -+will not perform any better than an unfiltered object walk; all reachable > -+objects are walked in order to populate the list. > ++able to populate an `omitted` list. This list of filtered objects may have > ++performance implications, however, because despite filtering objects, the possibly > ++much larger set of all reachable objects must be processed in order to > ++populate that list. > > First, add the `struct oidset` and related items we will use to iterate it: > > 5: c571abb49d = 5: 8920313ee2 MyFirstObjectWalk: add stderr to pipe processing > -- > 2.43.0 >
