Re: [PATCH v2 2/3] receive-pack: skip connectivity checks on delete-only commands
- From
Patrick Steinhardt <ps@pks.im>
- Date
- Jun 29, 2021, 06:26 UTC
- Message-ID
- <YNq8+Jlsu1UfDKav@ncase>
- In-Reply-To
- <87y2auwh1d.fsf@evledraar.gmail.com>
On Mon, Jun 28, 2021 at 10:00:26AM +0200, Ævar Arnfjörð Bjarmason wrote:
Show 12 quoted lines
> > 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'.
Show 33 quoted lines
> > [...] > > 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