Re: [PATCH] patch-ids.c: cache patch IDs in a notes tree

From: John Keeping <john@keeping.me.uk>
Date: May 12, 2013, 09:08 UTC
Message-ID: <20130512090850.GH2299@serenity.lan>
In-Reply-To: <alpine.DEB.1.00.1305111846160.8213@s15462909.onlinehome-server.info>

On Sat, May 11, 2013 at 06:57:02PM -0500, Johannes Schindelin wrote:

Show 16 quoted lines

> On Sat, 11 May 2013, Linus Torvalds wrote:
> 
> > [...] I really think caching patch ID's should be something people
> > should be aware of is fundamentally wrong, even if it might work.
> 
> Given the incredible performance win, I would say it is still worth
> looking into.
> 
> If you store also a hash of Git version and diff options (may even be the
> hash of the raw bytes of the diff options if you do not plan to share the
> ref between machines) with the patch ID, you can make it safe.
> 
> That hash would be generated at patch_id init time and
> load_cached_patch_id() would check this hash in addition to the return
> value of get_sha1() (and ignore the note if the version/diff options
> differ).

I was thinking about this overnight, glad to see someone else had the same idea :-)

It's slightly annoying because the diff options can be customized after we return from init_patch_ids() so we either need a new setup_patch_ids() function to be run after init once diff options have been set or to set it lazily. I'll try introducing a setup function.

> If you are following git.git slavishly, maybe hashing just the major/minor
> Git version would be in order to avoid frequent regeneration of identical
> patch IDs.

I think just storing the version is quite good here, and it avoids pain when a topic that affects patch IDs is working its way through pu and next.

Show 8 quoted lines

> > And quite frankly, if you do rebases etc so much that you think patch
> > ID's are so important that they need to be cached, you may be doing
> > odd/wrong things.
> 
> AFAICT John actually gave a very valid scenario that validates his use
> case: git-gui patches are best tested in the git.git scenario but have to
> be contributed via git-gui.git. It's not John's fault that this typically
> requires a lot of rebasing between vastly divergent histories.

Actually, I don't think that use case is valid. Because it's a subtree merge I can be absolutely certain that nothing on the LHS of master...git-gui/master is patch identical to anything on the RHS since all the paths are different. So doing "git log --cherry-mark" in that case is completely useless. I think my script should be able to learn that, which gets rid of the really horrible case I was seeing, but it would be nice to improve the "fast enough" cases as well if it can be done without too much effort.