On Fri, Nov 09 2018, Duy Nguyen wrote: > On Fri, Nov 9, 2018 at 2:46 PM Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx> wrote: >> I'm planning to re-submit mine with some minor changes after the great >> Documentation/config* move lands. >> >> As noted in >> https://public-inbox.org/git/87bm7clf4o.fsf@xxxxxxxxxxxxxxxxxxx/ and >> https://public-inbox.org/git/87h8gq5zmc.fsf@xxxxxxxxxxxxxxxxxxx/ I think >> it's regardless of Jeff's optimization is. O(nothing) is always faster >> than O(something), particularly (as explained in that E-Mail) on NFS. > > Is it really worth adding more code to maintain just to shave a couple > seconds (or a few percent clone time)? Yeah I think so, because (in rough priority order): a) The maintenance burden of carrying core.checkCollisions is trivial, and it's hard to imagine a scenario where it'll be difficult to selectively turn off some does_this_collide() function. b) I think I need to worry more about a meteorite colliding with the datacenter than the threat this check is trying to guard against. c) I think we should just turn it off by default on SHA-1, but don't expect that argument will carry the day. But I expect even those who think we still need it will have a hard time making that argument in the case of SHA-256. So having the codepath to disable it is helpful. d) As shown in the linked E-Mails of mine you sometimes pay a 2-3 second *fixed* cost even for a very small (think ~100-200 objects) push/fetch that would otherwise take milliseconds with Jeff's version of this optimization (and not with mine). This can be a hundred/thousands of percent slowdown. Is that a big deal in itself in terms of absolute time spent? No. But I'm also thinking about this from the perspective of getting noise out of performance metrics. Some of this slowdown is also "user waiting for the terminal to be usable again" not just some machine somewhere wasting its own time. e) As shown in the patch I have this direction as a very beneficial side-effect makes it much easier to repair corrupt repositories. Something I'm hoping to pursue even further. I've had cases where core.checkCollisions=false + stuff on top would have made repairing a broken repo much easier. Anyway, I'm in no rush to send my patch. I'm happily using it in production, but will wait for Jeff's be ready and to land before picking it up again. Just wanted to do a braindump of the benefits.
