On Thu, Nov 08 2018, Ævar Arnfjörð Bjarmason wrote: > On Thu, Nov 08 2018, Jeff King wrote: > >> On Wed, Nov 07, 2018 at 10:55:24PM +0000, Geert Jansen wrote: >> >>> On Mon, Oct 29, 2018 at 07:27:39PM -0400, Jeff King wrote: >>> >>> > On Mon, Oct 29, 2018 at 08:36:07PM +0100, Ævar Arnfjörð Bjarmason wrote: >>> > > * Re-roll my 4 patch series to include the patch you have in >>> > > <20181027093300.GA23974@xxxxxxxxxxxxxxxxxxxxx> >>> > >>> > I don't think it's quite ready for inclusion as-is. I hope to brush it >>> > up a bit, but I have quite a backlog of stuff to review, as well. >>> >>> We're still quite keen to get this patch included. Is there anything I can do >>> to help? >> >> Yes, testing and review. :) >> >> I won't send the series out just yet, as I suspect it could use another >> read-through on my part. But if you want to peek at it or try some >> timings, it's available at: >> >> https://github.com/peff/git jk/loose-cache > > Just a comment on this from the series: > > Note that it is possible for this to actually be _slower_. We'll do a > full readdir() to fill the cache, so if you have a very large number of > loose objects and a very small number of lookups, that readdir() may end > up more expensive. > > In practice, though, having a large number of loose objects is already a > performance problem, which should be fixed by repacking or pruning via > git-gc. So on balance, this should be a good tradeoff. > > Our biggest repo has a very large number of loose objects at any given > time, but the vast majority of these are because gc *is* happening very > frequently and the default expiry policy of 2wks is in effect. > > Having a large number of loose objects is not per-se a performance > problem. > > It's a problem if you end up "faulting" to from packs to the loose > object directory a lot because those objects are still reachable, but if > they're not reachable that number can grow very large if your ref churn > is large (so lots of expired loose object production). > > Anyway, the series per-se looks good to me. It's particularly nice to > have some of the ODB cleanup + cleanup in fetch-pack.c > > Just wanted to note that in our default (reasonable) config we do > produce scenarios where this change can still be somewhat pathological, > so I'm still interested in disabling it entirely given the > implausibility of what it's guarding against. Some actual numbers for this for a fairly small repo on NFS, "cold" cache (hadn't run it in a bit): $ time (find objects/?? -type f|wc -l) 862 real 0m1.927s Warm cache: $ time (find objects/?? -type f|wc -l) 872 real 0m0.151s Cold cache on a bigger monorepo: $ time (find objects/?? -type f|wc -l) real 0m4.336s Warm cache on a bigger monorepo (more ref churn): $ time (find objects/?? -type f|wc -l) 49746 real 0m1.082s This on a server where bulk sustained writes of large files are really fast (up to 1GB/s). It's just these metadata ops that are slow. I also get cold cache times of up to 6 seconds on: time (find $(ls -d objects/??|sort -R) -type f | wc -l) As opposed max of ~4s without -R, so I suspect there may be some client/server optimization where things are iterated over in recursive glob order (pre-fetched?), whereas the cache will try to fill buckets is it encounters loose objects, so iterate over objects/{00..ff} randomly. I'm not really leading up to any point here I haven't made already. I was just curious to try to find some upper bound of overhead if say a pack with 512 objects is pushed. In that case it's very likely that we need to fill at least 200/256 buckets.
