On Tue, Jun 20, 2023 at 10:22:17AM -0400, Taylor Blau wrote: > In a similar fashion as a previous commit, teach `upload-pack` to avoid > enumerating hidden references where possible. > > Note, however, that there are certain cases where cannot avoid > enumerating even hidden references, in particular when either of: > > - `uploadpack.allowTipSHA1InWant`, or > - `uploadpack.allowReachableSHA1InWant` > > are set, corresponding to `ALLOW_TIP_SHA1` and `ALLOW_REACHABLE_SHA1`, > respectively. > > When either of these bits are set, upload-pack's `is_our_ref()` function > needs to consider the `HIDDEN_REF` bit of the referent's object flags. > So we must visit all references, including the hidden ones, in order to > mark their referents with the `HIDDEN_REF` bit. > > When neither `ALLOW_TIP_SHA1` nor `ALLOW_REACHABLE_SHA1` are set, the > `is_our_ref()` function considers only the `OUR_REF` bit, and not the > `HIDDEN_REF` one. `OUR_REF` is applied via `mark_our_ref()`, and only > to objects at the tips of non-hidden references, so we do not need to > visit hidden references in this case. Both of these are noops if uploadpack.allowAnySHA1InWant is set, or if we are using the v2 protocol. Setting both allowAny and allowTip is sufficiently stupid that I don't care much that they lose out on an optimization. But I could see somebody setting one of those for the benefit of v0 clients, but then also serving v2 clients (which effectively ignore those restrictions). I guess v2 clients don't hit this code at all (they are handled by ls-refs, which comes in the next patch). So that just leaves the case that allowAny is set. By itself the optimization should kick in (good). With allowTip or allowReachable it would not, but that is the "this is stupid" case in which it is OK to fall back to the existing behavior (even though we _could_ enable the optimization). OTOH, it would be easy to check it, as it's just another bit in allow_uor. I'm OK going either way. -Peff
