Re: Rebase performance

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

 



On Thu, Feb 25, 2016 at 10:02 AM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Duy Nguyen <pclouds@xxxxxxxxx> writes:
>
>> On Thu, Feb 25, 2016 at 5:09 AM, Christian Couder
>> <christian.couder@xxxxxxxxx> wrote:
>>> Another possibility would be to libify the "git apply" functionality
>>> and then to use the libified "git apply" in run_apply() instead of
>>> launching a separate "git apply" process. One benefit from this is
>>> that we could probably get rid of the read_cache_from() call at the
>>> end of run_apply() and this would likely further speed up things. Also
>>> avoiding to launch separate processes might be a win especially on
>>> Windows.
>>
>> The amount of global variables in apply.c is just scary. Libification
>> will need some cleanup first (i'm not against it though).
>
> The global variables do not scare me.  You can just throw them into
> a single "apply_state" structure and pass it around the callchain
> just fine--the helper functions in the file all take only a small
> number of parameters, and a new parameter that is a pointer to the
> state structure that consistently comes at the beginning would not
> make things harder to read or maintain.

There are a bunch of shadow variables in this file. If you are not
careful, it's easy to mistake local var "x" with global "x" and
replace it with global_state->x.

> What does scare me (and what you should be scared) more is the way
> the current code handles erroneous input.  It was one of the
> programs written in early days with "just do one thing well and
> exit, the larger program structure will be scripted around us"
> mentality and liberally die() without cleaning things up.  I do
> agree with others that libification of it is a very good thing, but
> the second step (the first step is the easier "global to state
> structure" one, which should be more or less mechanical) towards it
> is to design how the errors are handled and resources are released.

Yeah.. thorough study of apply code may be needed before anybody does
any surgery in this code
-- 
Duy
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[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]