Re: [PATCH] Remove StGit patch detection from vcs_info

["Peter Grayson" <pete@xxxxxxxxxxxxx>]

On Mon, Nov 14, 2022, at 8:15 AM, Daniel Shahaf wrote:
> Peter Grayson wrote on Sun, Nov 13, 2022 at 22:38:50 -0500:
>> On Sat, Nov 12, 2022, at 11:30 PM, Daniel Shahaf wrote:
>> > What's the impact on people who don't have stg(1) installed, or who have
>> > stg(1) installed but are currently in a worktree that doesn't use StGit?
>> > I.e., are those figures immediately after `git init`, or in a worktree
>> > that has a StGit patch stack, or?
>> 
>> Without stg(1) installed, the cost would be however long it takes zsh to
>> determine that the executable is not available in $path, which is
>> ostensibly very fast (microseconds?).
>> 
>> If stg(1) is installed, but run in a repo with a branch that has not been
>> initialized with `stg init`, it's still about 12ms. Almost all that time
>> is taken just to initialize a libgit2 Repository structure, which is
>> used to interrogate the object database to determine whether a StGit
>> stack is initialized.
>> 
>
> I see.
>
>> > In general, 32ms for everyone might be too much (what if another
>> > third-party tool wanted to do its own elif branch that also spent 32ms
>> > for everyone?  That'd be 64ms in total and counting…).
>> 
>> Well, that was my original thought as well and why my first patch simply
>> removed StGit support from vcs_info. Note that 32ms is on a particularly
>> fast machine and I stand by my assertion that Python startup time can be
>> on the order of hundreds of milliseconds on more modest machines and/or
>> with older versions of Python.
>> 
>
> *nod*
>
>> > However, if the 32ms would only be seen by users of an EOL'd stg(1),
>> > we can relax the threshold a bit.  On the other hand, if it's 32ms
>> > just in cases where stg(1) is used… well, there doesn't seem to be
>> > much we /can/ do there.
>> 
>> The relevant scenarios:
>> 
>> * stg(1) not installed => ~free
>> * stg(1) < 2.0 installed => 30+ ms
>> * stg(1) >= 2.0 installed => ~10 ms
>> 
>> The worst case scenario would be for someone where stg(1) <2.0 is
>> installed, but not used. This would create overhead in vcs_info without
>> any value for the user.
>
> Yeah, the majority of git users don't use stg, so let's try not to make
> them wait an extra 30ms for every prompt.
>
> Fortunately, that delay only happens with EOL versions of stg(1).  That
> gives us some leeway in our design choices.  Thinking out loud:
>
> How likely is it for a vcs_info git user to have stg(1) 0.x or 1.x
> installed and /not/ use it?
>
> Unfortunately, the prior probability of having 0.x installed isn't
> exactly low, since 0.x is packaged in Debian and derivatives and they
> don't seem likely to upgrade soon (see the inactivity on
> <https://bugs.debian.org/995869>).  That's aside from how quick distros
> may or may not be on upgradomg from 1.x (so far only Homebrew and Arch's
> AUR have it, according to repology).
>
> Next, in case 0.x or 1.x is installed, for it to be unused seems to mean
> the OS installation is either shared (more than one human user) or
> has a single user who uses stg(1) only in some of their repositories.
>
> In the former case, the box is likely relatively performant… but so is
> the box you took the measurements on.  In the latter case, opt-in zstyle
> settings with repo-root-name in the context patterns could be used to
> selectively enable/disable stg(1) use… but that's hardly ideal.
>
> All of this only affects people with an EOL version of stg(1) installed,
> but since it affects people who /don't/ use stg(1), we can't really say
> "Patches welcome" to the affected people, can we.
>
> It seems the better course of action here is to get distros to upgrade
> to 2.x, so the problem situation doesn't arise in the first place.
>
> Until that happens, how about making VCS_INFO_get_data_git integrate
> only with stg ≥2.x by default, citing the EOL status and performance
> concerns?  That is, guard the code being added by «if [[ stg version is
> 2.x or newer ]] || user has opted in».
>
> For the first disjunct, normally we would prefer «stg --version», but
> that likewise takes 30ms here (stg 0.x in a chroot), so I reckon we'll
> want some other approach.  Any ideas?  I've thought of two options, but
> they're both hacky and I'm sure there are much better ones.  (For the
> record, they are to check whether «=stg» is a Python script and to check
> whether it's smaller than 2KB.)

Another option would be to have zsh invoke `git rev-parse
refs/stacks/$gitbranch` to test whether a StGit stack is initialized on
the current branch. This takes less than half a millisecond. We could
pair this with testing for the existence of the stg executable to make
it even faster. The downside of this approach is that it leaks StGit
internals into zsh, but this is a pretty small leak and zsh would still
use `stg series` to actually get the patch lists.

This seems like a good way forward to me.

>> All other scenarios seem to end up with the user getting what they
>> paid for.
>
> Is there any chance of optimizing the 10ms case as well?
>
> For instance, consider a has-stg(1) executable (or shell function) that
> returns 0 if the Git repository in cwd has an initialized StGit stack
> and returns 1 otherwise.  Would it be possible for has-stg to run faster
> than 10ms?  If so, vcs_info could use this (unconditionally if has-stg
> were maintained and shipped as part of StGit; otherwise, conditional on
> «use-simple» due to the layering violation).  WDYT?

I've been thinking about this. For the simplest cases of `stg series`,
the stack metadata needed to service the command could be obtained using
`git cat-file -p refs/stacks/$gitbranch:stack.json` which takes about
0.5ms versus the libgit2 initialization of 11ms. So an optimized `stg
series --noprefix --applied` would be closer to 1ms. This same approach
could be applied to several stg subcommands, so it wouldn't necessarily
be a one-off optimization.

Further down the road, I have hopes of the gitoxide[1] project maturing
enough to replace libgit2 in StGit.

[1] https://github.com/Byron/gitoxide

>> > Perhaps hide some or all of the work behind an opt-in switch.  (For
>> > instance, the default settings show only the topmost applied patch, so
>> > if it's possible to tell stg(1) not to emit any other patches, that
>> > might help the code finish more quickly.)
>> 
>> Limiting the output to only the topmost patch will not have a material
>> effect on the runtime because of how the overhead is dominated by
>> libgit2 initialization. So an opt-in switch would have be all or
>> nothing; either try to run stg(1) or not.
>> 
>> Also worth noting is that changing StGit support in vcs_info opt-in
>> would be a bit of a regression and would add more configuration
>> surface area. Not sure if/how the zsh project does versioning for
>> something like this. I did not include such a switch in my latest
>> patch because of these issues.
>> 
>
> When we make an incompatible change, we document it in the list of
> incompatibilities in our NEWS/README files.  We can consider doing more
> than that (e.g., printing a message at runtime informing the user of the
> opt-in) as appropriate.
>
> In this instance, since released zsh only supports StGit 0.x,
> introducing an opt-in would be a regression only for users who upgrade
> to zsh 5.10 not after upgrading stg to 1.x or 2.x — i.e., typically,
> users of LTS distros as opposed to rolling distros.
>
> That being the case, perhaps adding a paragraph to the list of
> incompatibilities in NEWS/README would suffice?  Upgrading an LTS distro
> does usually involve reading through a long list of incompatible changes
> that affect some users only.
>
> Alternatively, perhaps add a runtime message along the lines of:
> .
>     if (( ! OPTED_OUT_OF_THIS_MESSAGE )) &&
>        type stg > /dev/null &&
>        [[ -z ${(M)${(f)$(git for-each-ref)}:#*stgit*} ]]
>     then
>         # tell the user they may want to set the opt-in
>     fi
> .
> , though I wonder now whether that last conjunct is fast enough…

I'm going to take a shot at making the StGit stack detection cheap enough
that vcs_info can just merrily support StGit without configuration
switches, user-facing messages, or incompatibility notices.

Thanks again for your detailed consideration of this topic.

Pete