[PATCH 4 of 4 NEW-CONCEPT] track-tags: write all tag changes to a file
quark at fb.com
Thu Apr 6 14:18:44 EDT 2017
Thanks for the clarification.
My concern about performance is not writing the file, but calculating the
content to be written. I'd like to see more clarification about whether and
when this new code path triggers a full manifest read, since reading a flat
manifest is painfully slow on a large repo.
Excerpts from Pierre-Yves David's message of 2017-04-06 08:33:12 +0200:
> On 04/06/2017 01:23 AM, Jun Wu wrote:
> > Excerpts from Pierre-Yves David's message of 2017-04-06 01:09:47 +0200:
> >> On 04/05/2017 10:54 PM, Jun Wu wrote:
> >>> I don't think "writing things that hook *may* need to filesystem" is a good
> >>> approach. It introduces unnecessary overhead if the hook does not need that
> >>> information.
> >> I do not find the overhead concerning.
> > This is from your commit message:
> > This code is disabled by default as there is still various performance
> > concerns. Some require a smarter use of our existing tag caches and some
> > other require rework around the transaction logic to skip execution when
> > unneeded. These performance improvement have been delayed, I would like
> > to be able to experiment and stabilize the feature behavior first.
> The part about "skip execution when unneeded" refers to the equivalent
> of the "pending" feature of the transaction. If no hook exists, the
> pending transaction data are not made available to hook. I think it make
> senses to mirror this behavior for the "changes/" data. But I don't
> think is it worth being more discriminatory in what we write once we
> start writing. They are no such discrimination for the "pending" data.
> All the rest is about the implementation needing some known optimization
> before it gets out of experimental. Nothing special or relevant long
> term there.
> >> The thing to keep in mind is that if they are many things to write down,
> >> this also means many thing changed during the transaction. So the
> >> overhead is likely minimal. In the tags cases, just updating the various
> >> tags related cache is going to be much more expensive that writing this
> >> to disk.
> >>> I think a better way is to have per-hook config to let hooks explicitly
> >>> declare what they need, before we run them.
> >> I afraid it is too fragile. Forgetting to configure the hook properly
> >> will be easy and the error will likely pass silently as hook will run
> >> normally with the same data as if the transaction touched no tags.
> > Do you mean that a global config option is less fragile?
> What global option are you talking about? The feature is currently gated
> behind an experimental flag until the cache usage and transaction API is
> fixed. But that's not part of the final API (since this just an
> experimental gating).
More information about the Mercurial-devel