Re: [PATCH v2 2/3] receive-pack: skip connectivity checks on delete-only commands

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 4233 bytes --]

On Mon, Jun 28, 2021 at 10:00:26AM +0200, Ævar Arnfjörð Bjarmason wrote:
> 
> On Mon, Jun 28 2021, Patrick Steinhardt wrote:
> 
> > [[PGP Signed Part:Undecided]]
> > In the case where git-receive-pack(1) receives only commands which
> > delete references, then per technical specification the client MUST NOT
> > send a packfile. As a result, we know that no new objects have been
> > received, which makes it a moot point to check whether all received
> > objects are fully connected.
> 
> Is it just per specification, or do we also have assertions/tests for
> what happens in that case?

I'm not sure whether we have any tests for this, but I've seen several
hangs already in case the server did expect a packfile or errors in case
the client sent one. In any case, the technical specification in
Documentation/technical/pack-protocol.txt is quite clear on this:

    The packfile MUST NOT be sent if the only command used is 'delete'.

> > [...]
> > The following tests were executed on linux.git and back up above
> > expectation:
> >
> > Test                                                  origin/master           HEAD
> > ---------------------------------------------------------------------------------------------------------
> > 5400.4: empty receive-pack updated:new                178.36(428.22+164.36)   177.62(421.33+164.48) -0.4%
> > 5400.7: clone receive-pack updated:new                0.10(0.08+0.02)         0.10(0.08+0.02) +0.0%
> > 5400.9: clone receive-pack updated:main               0.10(0.08+0.02)         0.11(0.08+0.02) +10.0%
> > 5400.11: clone receive-pack main~10:main              0.15(0.11+0.04)         0.15(0.10+0.05) +0.0%
> > 5400.13: clone receive-pack :main                     0.01(0.00+0.01)         0.01(0.01+0.00) +0.0%
> > 5400.16: clone_bitmap receive-pack updated:new        0.10(0.07+0.02)         0.09(0.06+0.02) -10.0%
> > 5400.18: clone_bitmap receive-pack updated:main       0.10(0.07+0.02)         0.10(0.08+0.02) +0.0%
> > 5400.20: clone_bitmap receive-pack main~10:main       0.15(0.11+0.03)         0.15(0.12+0.03) +0.0%
> > 5400.22: clone_bitmap receive-pack :main              0.02(0.01+0.01)         0.01(0.00+0.00) -50.0%
> > 5400.25: extrarefs receive-pack updated:new           32.34(20.72+11.86)      32.56(20.82+11.95) +0.7%
> > 5400.27: extrarefs receive-pack updated:main          32.42(21.02+11.61)      32.52(20.64+12.10) +0.3%
> > 5400.29: extrarefs receive-pack main~10:main          32.53(20.74+12.01)      32.39(20.63+11.97) -0.4%
> > 5400.31: extrarefs receive-pack :main                 7.13(3.53+3.59)         7.15(3.80+3.34) +0.3%
> > 5400.34: extrarefs_bitmap receive-pack updated:new    32.55(20.72+12.04)      32.65(20.68+12.18) +0.3%
> > 5400.36: extrarefs_bitmap receive-pack updated:main   32.50(20.90+11.86)      32.67(20.93+11.94) +0.5%
> > 5400.38: extrarefs_bitmap receive-pack main~10:main   32.43(20.88+11.75)      32.35(20.68+11.89) -0.2%
> > 5400.40: extrarefs_bitmap receive-pack :main          7.21(3.58+3.63)         7.18(3.61+3.57) -0.4%
> 
> We're doing less work so I'd expect to te be faster, but do these tests
> really back that up? From eyeballing these I can't find a line where the
> confidence intervals don't overlap, e.g. the +10% regresison is a
> .10->.11 "regression" with a [+-] 0.02 (so within the error bars) etc,
> ditto for the -50% improvement.
> 
> Perhaps the error bars will reduce with a high GIT_PERF_REPEAT_COUNT, or
> the re-arrangement for keeping things hotter in cache that I suggested
> in 1/3.

As I've layed out in the commit message, all we save now is spawning
git-rev-list(1). The command list iterator which is used to feed data
into git-rev-list(1) wouldn't provide any references given that it
skips over all queued updates whose new OID is the null OID. So
git-rev-list(1) doesn't receive any input except `--not --all` and thus
can exit without doing a graph walk.

Above numbers simply show that this saving is not significant and gets
lost in the noise, at least on Linux. Windows may show slightly
different numbers given that spawning of processes is slower there, but
I don't expect it to matter much there, either.

Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]