Re: [PATCH v1 2/2] merge: Add merge.aggressive config setting

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 4/20/2018 1:22 PM, Elijah Newren wrote:
On Fri, Apr 20, 2018 at 6:36 AM, Ben Peart <Ben.Peart@xxxxxxxxxxxxx> wrote:
Add the ability to control the aggressive flag passed to read-tree via a config setting.

This feels like a workaround to the performance problems with index
updates in merge-recursive.c.

This change wasn't done to solve performance problems. We turned it on because it reduced the number of unmerged entries (from 40K to 1) in the particular merge we were looking at. The additional 3 scenarios that --aggressive resolves made that much difference.

That said, it makes sense to me to do
this when rename detection is turned off.  In fact, I think you'd
automatically want to set aggressive to true whenever rename detection
is turned off (whether by your merge.renames option or the
-Xno-renames flag).
> I can't think of any reason this setting would be useful separate from
turning rename detection off, and it'd actively harm rename detection
performance improvements I have in the pipeline.  I'd really prefer to
not add this option, and instead combine the setting of aggressive
with the other flag.  Do you have an independent reason for wanting
this?


While combining them would work for our specific use scenario (since we turn both on already along with turning off merge.stat), I really hesitate to tie these two different flags and code paths together with a single config setting.

While I don't want to needlessly complicate your optimizations in this area (they are already complex enough!) I believe we need to keep the option to turn on --aggressive without turning off rename detection as a viable option. Perhaps if that is the case, your optimizations have less impact or don't apply but the user should be able to make that choice for their specific situation.

Thanks,
Elijah




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux