On 3/17/2022 11:55 AM, Victoria Dye via GitGitGadget wrote: > To fix this, the 'cache_bottom' advancement is reinstated in > 'mark_ce_used(...)', and instead it is disabled in 'unpack_callback(...)' if > the tree in question is a sparse directory. This corrects both the > non-"cache diff" cases and the 'unpack_index_entry(...)' cases while > preventing the double-advancement 17a1bb570b originally intended to avoid > (patch [2/3]). Thank you for digging deep and finding the root cause here _and_ the correct fix. > Finally, now that the cache bottom is advanced properly, we can revert the > "performance improvement" introduced in f2a454e0a5 (unpack-trees: improve > performance of next_cache_entry, 2021-11-29) that mitigated performance > issues arising in 'next_cache_entry(...)' from the non-advancing > 'cache_bottom' (patch [3/3]). The performance results in > 'p2000-sparse-operations.sh' showed expected variability around 0% change in > execution time (+/= 0.04s, depending on the command), with example results > for potentially-affected commands below. > > 'git reset' master this_series > ------------------------------------------------------------------------ > full-v3 0.51(0.21+0.27) 0.50(0.21+0.25) -2.0% > full-v4 0.51(0.22+0.27) 0.50(0.21+0.24) -2.0% > sparse-v3 0.30(0.04+0.55) 0.28(0.04+0.50) -6.7% > sparse-v4 0.31(0.04+0.51) 0.29(0.04+0.51) -6.5% > > 'git reset -- does-not-exist' master this_series > ------------------------------------------------------------------------ > full-v3 0.54(0.23+0.27) 0.55(0.22+0.28) +1.9% > full-v4 0.56(0.25+0.26) 0.54(0.24+0.26) -3.6% > sparse-v3 0.31(0.04+0.54) 0.31(0.04+0.50) +0.0% > sparse-v4 0.31(0.04+0.52) 0.31(0.04+0.50) +0.0% > > 'git diff --cached' master this_series > ------------------------------------------------------------------------- > full-v3 0.09(0.04+0.04) 0.09(0.04+0.04) +0.0% > full-v4 0.09(0.04+0.04) 0.09(0.04+0.04) +0.0% > sparse-v3 0.05(0.01+0.02) 0.05(0.01+0.03) +0.0% > sparse-v4 0.04(0.01+0.02) 0.04(0.01+0.02) +0.0% I'm glad this also makes things simpler here. It's interesting that it previously manifested only as a performance issue and not a correctness issue. I carefully read these patches and think they are ready to go. Thanks, -Stolee
