Re: [PATCH] Remove StGit patch detection from vcs_info

[Daniel Shahaf <d.s@xxxxxxxxxxxxxxxxxx>]

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.)

> 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?

> > 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…

> > From the "Is your computer plugged in?" department: Is that 32ms figure
> > on hot disk cache with .pyc files already having been generated?
> 
> Yes. I used hyperfine(1) to do these measurements.

*nod*

Cheers,

Daniel