Re: [PATCH v2 7/7] merge-ort: restart merge with cached renames to reduce process entry cost

[Derrick Stolee <stolee@gmail.com>]

On 7/15/2021 12:53 PM, Elijah Newren wrote:
> On Thu, Jul 15, 2021 at 8:10 AM Derrick Stolee <stolee@gmail.com> wrote:
>>
>> On 7/13/2021 3:33 PM, Elijah Newren via GitGitGadget wrote:
>>> From: Elijah Newren <newren@gmail.com>
...
>>> +              * The exact number isn't critical, since the code will
>>> +              * work even if we get the factor wrong -- it just might be
>>> +              * slightly slower if we're a bit off.  For now, just error
>>> +              * on the side of a bigger fudge.  For the linux kernel
>>
>> super-nit: s/linux/Linux/
> 
> Yeah, I tend to refer to projects by the name of their repository
> instead of their proper name.  (I do it with git too.)  Bad habit.
> Will fix.  That is, I will fix this instance.  Not sure I can fix the
> habit.

If you had written it as torvalds/linux, then I wouldn't have batted
an eye, because that is clearly a repo name (at least, clear to me).

>>> +              * testcases I was looking at with massive renames, the
>>> +              * ratio came in around 50 to 250, which clearly would
>>> +              * trigger this optimization and provided some *very* nice
>>> +              * speedups.
>>
>> This bit of your testing might be more appropriate for your commit
>> message. This discussion of a test made at a certain point in time
>> is more likely to go stale than the description of how this factor
>> does not change correctness, only performance.
> 
> The commit message does include discussion on how this factor only
> changes performance, not correctness.  I left this comment in the code
> because I figured it looked weird and magic and deserved an
> explanation without resorting to git-log or git-blame.  Granted, I
> wrote this comment and the commit message at much different times (I
> wrote the comment first, then the commit message many months later)
> and thus summarized a bit differently.  But I thought I had the same
> relevant content in both places.
> 
> Are there pieces you feel are missing from the commit message?  Should
> I trim this comment down in the code and just let people look for the
> commit message for more details?

I meant to say "these kinds of details are better for the commit
message instead of comments" and not say "your commit message
doesn't have enough."

I don't feel strongly about this being present in the comment or
not, but it seems like something that could be dropped from the
comment without loss of information.

Thanks,
-Stolee