Re: [PATCH] defer expensive load_ref_decorations until needed
- From
Phil Hord <phil.hord@gmail.com>
- Date
- Nov 22, 2017, 17:45 UTC
- Message-ID
- <CABURp0rNfdXaXH-meRvM+mjf+ucHKfePjDU7ZGKt0ug3wOanhA@mail.gmail.com>
- In-Reply-To
- <xmqqk1yiu9fo.fsf@gitster.mtv.corp.google.com>
On Tue, Nov 21, 2017, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> Junio C Hamano <gitster@pobox.com> writes: > > I am not sure if "maybe_" is a good name here, though. If anything, > you are making the semantics of "load_ref_decorations()" to "maybe" > (but I do not suggest renaming that one). > > How about calling it to load_ref_decorations_lazily() or something?
I groped about for something conventional, but "..._gently" didn't fit the bill, so I went with "maybe". I like "lazily" better for this case. I will change it for v2.
Show 12 quoted lines
>> Other than that, I like what this patch attempts to do. A nicely >> identified low-hanging fruit ;-). > > Having said that, this will have a bad interaction with another > topic in flight: <20171121213341.13939-1-rafa.almas@gmail.com> > > Perhaps this should wait until the other topic lands and stabilizes. > We'd need to rethink if the approach taken by this patch, i.e. to > still pass the info to load() but holding onto it until the time > lazy_load() actually uses it, is a sensible way forward, or we would > want to change the calling convention to help making it easier to > implement the lazy loading.
I noticed that after just after cleaning this one up, but didn't look closely yet. I'll hold this in my local queue until ra lands.
P