blame: enable and test the sparse index #431

ldennington · 2021-09-22T22:16:25Z

Enable the sparse index for the 'git blame' command. The index was already
not expanded with this command, so the most interesting thing to do is
to add tests that verify that 'git blame' behaves correctly when the sparse
index is enabled and that its performance improves. More specifically, these
cases are...

The index is not expanded for blame when given paths in the sparse
checkout cone at multiple levels.
Performance measurably improves for blame with sparse index when
given paths in the sparse checkout cone at multiple levels.

The following are the performance results comparing the branch with
sparse index enabled for blame with the vfs-2.33.0 branch in the
microsoft/git repository:

Test                                       msft/vfs-2.33.0   sparse-index-blame
-----------------------------------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)        0.31(0.21+0.09)   0.32(0.21+0.10) +3.2%
2000.3: git blame f2/f4/a (full-v4)        0.29(0.19+0.08)   0.31(0.21+0.08) +6.9%
2000.4: git blame f2/f4/a (sparse-v3)      0.55(0.38+0.14)   0.23(0.14+0.07) -58.2%
2000.5: git blame f2/f4/a (sparse-v4)      0.57(0.40+0.16)   0.23(0.15+0.07) -59.6%
2000.6: git blame f2/f4/f3/a (full-v3)     0.77(0.56+0.20)   0.85(0.60+0.22) +10.4%
2000.7: git blame f2/f4/f3/a (full-v4)     0.78(0.56+0.20)   0.81(0.59+0.21) +3.8%
2000.8: git blame f2/f4/f3/a (sparse-v3)   1.07(0.78+0.28)   0.72(0.52+0.19) -32.7%
2000.9: git blame f2/f4/f3/a (sparse-v4)   1.05(0.78+0.26)   0.73(0.51+0.19) -30.5%

We do not include paths outside the sparse checkout cone because blame
currently does not support blaming files outside of the sparse
definition.

derrickstolee · 2021-09-23T15:53:30Z

Test                                    f2bf7293c6     
-------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)     0.27(0.19+0.07)
2000.3: git blame f2/f4/a (full-v4)     0.27(0.18+0.07)
2000.4: git blame f2/f4/a (sparse-v3)   0.53(0.38+0.14)
2000.5: git blame f2/f4/a (sparse-v4)   0.53(0.37+0.15)

On Linux, I get these numbers:

Test                                    ms/vfs-2.33.0     sparse-index-blame    
--------------------------------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)     0.13(0.11+0.02)   0.13(0.10+0.03) +0.0% 
2000.3: git blame f2/f4/a (full-v4)     0.12(0.09+0.03)   0.12(0.10+0.02) +0.0% 
2000.4: git blame f2/f4/a (sparse-v3)   0.34(0.31+0.03)   0.08(0.06+0.01) -76.5%
2000.5: git blame f2/f4/a (sparse-v4)   0.34(0.31+0.02)   0.08(0.08+0.01) -76.5%

It might be one of those things where macOS is behaving differently. I'm going to deepen the tree and see how that looks.

Edit: Here are the numbers after increasing the depth by one:

$./run ms/vfs-2.33.0 sparse-index-blame -- p2000-sparse-operations.sh
...
Test                                       ms/vfs-2.33.0     sparse-index-blame    
-----------------------------------------------------------------------------------
2000.2: git blame f2/f4/f3/a (full-v3)     0.48(0.38+0.10)   0.47(0.35+0.12) -2.1% 
2000.3: git blame f2/f4/f3/a (full-v4)     0.48(0.39+0.09)   0.47(0.38+0.08) -2.1% 
2000.4: git blame f2/f4/f3/a (sparse-v3)   1.26(1.17+0.09)   0.28(0.25+0.03) -77.8%
2000.5: git blame f2/f4/f3/a (sparse-v4)   1.27(1.11+0.15)   0.28(0.25+0.03) -78.0%

ldennington · 2021-09-23T17:21:16Z

Test                                    f2bf7293c6     
-------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)     0.27(0.19+0.07)
2000.3: git blame f2/f4/a (full-v4)     0.27(0.18+0.07)
2000.4: git blame f2/f4/a (sparse-v3)   0.53(0.38+0.14)
2000.5: git blame f2/f4/a (sparse-v4)   0.53(0.37+0.15)

On Linux, I get these numbers:

Test                                    ms/vfs-2.33.0     sparse-index-blame    
--------------------------------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)     0.13(0.11+0.02)   0.13(0.10+0.03) +0.0% 
2000.3: git blame f2/f4/a (full-v4)     0.12(0.09+0.03)   0.12(0.10+0.02) +0.0% 
2000.4: git blame f2/f4/a (sparse-v3)   0.34(0.31+0.03)   0.08(0.06+0.01) -76.5%
2000.5: git blame f2/f4/a (sparse-v4)   0.34(0.31+0.02)   0.08(0.08+0.01) -76.5%

It might be one of those things where macOS is behaving differently. I'm going to deepen the tree and see how that looks.

Edit: Here are the numbers after increasing the depth by one:

$./run ms/vfs-2.33.0 sparse-index-blame -- p2000-sparse-operations.sh
...
Test                                       ms/vfs-2.33.0     sparse-index-blame    
-----------------------------------------------------------------------------------
2000.2: git blame f2/f4/f3/a (full-v3)     0.48(0.38+0.10)   0.47(0.35+0.12) -2.1% 
2000.3: git blame f2/f4/f3/a (full-v4)     0.48(0.39+0.09)   0.47(0.38+0.08) -2.1% 
2000.4: git blame f2/f4/f3/a (sparse-v3)   1.26(1.17+0.09)   0.28(0.25+0.03) -77.8%
2000.5: git blame f2/f4/f3/a (sparse-v4)   1.27(1.11+0.15)   0.28(0.25+0.03) -78.0%

Ah this is very good to see! Let me try increasing the depth by 1 and doing the same comparison between ms/vfs-2.33.0 and my branch.

ldennington · 2021-09-23T17:33:49Z

Test                                    f2bf7293c6     
-------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)     0.27(0.19+0.07)
2000.3: git blame f2/f4/a (full-v4)     0.27(0.18+0.07)
2000.4: git blame f2/f4/a (sparse-v3)   0.53(0.38+0.14)
2000.5: git blame f2/f4/a (sparse-v4)   0.53(0.37+0.15)

On Linux, I get these numbers:

Test                                    ms/vfs-2.33.0     sparse-index-blame    
--------------------------------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)     0.13(0.11+0.02)   0.13(0.10+0.03) +0.0% 
2000.3: git blame f2/f4/a (full-v4)     0.12(0.09+0.03)   0.12(0.10+0.02) +0.0% 
2000.4: git blame f2/f4/a (sparse-v3)   0.34(0.31+0.03)   0.08(0.06+0.01) -76.5%
2000.5: git blame f2/f4/a (sparse-v4)   0.34(0.31+0.02)   0.08(0.08+0.01) -76.5%

It might be one of those things where macOS is behaving differently. I'm going to deepen the tree and see how that looks.
Edit: Here are the numbers after increasing the depth by one:

$./run ms/vfs-2.33.0 sparse-index-blame -- p2000-sparse-operations.sh
...
Test                                       ms/vfs-2.33.0     sparse-index-blame    
-----------------------------------------------------------------------------------
2000.2: git blame f2/f4/f3/a (full-v3)     0.48(0.38+0.10)   0.47(0.35+0.12) -2.1% 
2000.3: git blame f2/f4/f3/a (full-v4)     0.48(0.39+0.09)   0.47(0.38+0.08) -2.1% 
2000.4: git blame f2/f4/f3/a (sparse-v3)   1.26(1.17+0.09)   0.28(0.25+0.03) -77.8%
2000.5: git blame f2/f4/f3/a (sparse-v4)   1.27(1.11+0.15)   0.28(0.25+0.03) -78.0%

Ah this is very good to see! Let me try increasing the depth by 1 and doing the same comparison between ms/vfs-2.33.0 and my branch.

This is looking a lot better

Test                                       msft/vfs-2.33.0   sparse-index-blame    
-----------------------------------------------------------------------------------
2000.2: git blame f2/f4/f3/a (full-v3)     0.76(0.55+0.19)   0.87(0.63+0.23) +14.5%
2000.3: git blame f2/f4/f3/a (full-v4)     0.75(0.55+0.18)   0.87(0.63+0.22) +16.0%
2000.4: git blame f2/f4/f3/a (sparse-v3)   1.01(0.73+0.26)   0.77(0.55+0.20) -23.8%
2000.5: git blame f2/f4/f3/a (sparse-v4)   0.97(0.70+0.25)   0.72(0.52+0.19) -25.8%

ldennington · 2021-09-23T17:40:12Z

Test                                    f2bf7293c6     
-------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)     0.27(0.19+0.07)
2000.3: git blame f2/f4/a (full-v4)     0.27(0.18+0.07)
2000.4: git blame f2/f4/a (sparse-v3)   0.53(0.38+0.14)
2000.5: git blame f2/f4/a (sparse-v4)   0.53(0.37+0.15)

On Linux, I get these numbers:

Test                                    ms/vfs-2.33.0     sparse-index-blame    
--------------------------------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)     0.13(0.11+0.02)   0.13(0.10+0.03) +0.0% 
2000.3: git blame f2/f4/a (full-v4)     0.12(0.09+0.03)   0.12(0.10+0.02) +0.0% 
2000.4: git blame f2/f4/a (sparse-v3)   0.34(0.31+0.03)   0.08(0.06+0.01) -76.5%
2000.5: git blame f2/f4/a (sparse-v4)   0.34(0.31+0.02)   0.08(0.08+0.01) -76.5%

It might be one of those things where macOS is behaving differently. I'm going to deepen the tree and see how that looks.
Edit: Here are the numbers after increasing the depth by one:

$./run ms/vfs-2.33.0 sparse-index-blame -- p2000-sparse-operations.sh
...
Test                                       ms/vfs-2.33.0     sparse-index-blame    
-----------------------------------------------------------------------------------
2000.2: git blame f2/f4/f3/a (full-v3)     0.48(0.38+0.10)   0.47(0.35+0.12) -2.1% 
2000.3: git blame f2/f4/f3/a (full-v4)     0.48(0.39+0.09)   0.47(0.38+0.08) -2.1% 
2000.4: git blame f2/f4/f3/a (sparse-v3)   1.26(1.17+0.09)   0.28(0.25+0.03) -77.8%
2000.5: git blame f2/f4/f3/a (sparse-v4)   1.27(1.11+0.15)   0.28(0.25+0.03) -78.0%

Ah this is very good to see! Let me try increasing the depth by 1 and doing the same comparison between ms/vfs-2.33.0 and my branch.

This is looking a lot better

Test                                       msft/vfs-2.33.0   sparse-index-blame    
-----------------------------------------------------------------------------------
2000.2: git blame f2/f4/f3/a (full-v3)     0.76(0.55+0.19)   0.87(0.63+0.23) +14.5%
2000.3: git blame f2/f4/f3/a (full-v4)     0.75(0.55+0.18)   0.87(0.63+0.22) +16.0%
2000.4: git blame f2/f4/f3/a (sparse-v3)   1.01(0.73+0.26)   0.77(0.55+0.20) -23.8%
2000.5: git blame f2/f4/f3/a (sparse-v4)   0.97(0.70+0.25)   0.72(0.52+0.19) -25.8%

Yep, ok, looks like I was just holding it incorrectly

Test                                    msft/vfs-2.33.0   sparse-index-blame    
--------------------------------------------------------------------------------
2000.2: git blame f2/f4/a (full-v3)     0.27(0.18+0.07)   0.29(0.19+0.08) +7.4% 
2000.3: git blame f2/f4/a (full-v4)     0.27(0.18+0.07)   0.29(0.19+0.08) +7.4% 
2000.4: git blame f2/f4/a (sparse-v3)   0.53(0.37+0.14)   0.21(0.13+0.06) -60.4%
2000.5: git blame f2/f4/a (sparse-v4)   0.55(0.37+0.16)   0.21(0.14+0.06) -61.8%

derrickstolee

Approved with some minor edits recommended.

t/perf/p2000-sparse-operations.sh

builtin/blame.c

t/t1092-sparse-checkout-compatibility.sh

Enable the sparse index for the 'git blame' command. The index was already not expanded with this command, so the most interesting thing to do is to add tests that verify that 'git blame' behaves correctly when the sparse index is enabled and that its performance improves. More specifically, these cases are: 1. The index is not expanded for `blame` when given paths in the sparse checkout cone at multiple levels. 2. Performance measurably improves for `blame` with sparse index when given paths in the sparse checkout cone at multiple levels. The following are the performance results comparing the branch with sparse index enabled for `blame` with the vfs-2.33.0 branch in the microsoft/git repository: ``` Test msft/vfs-2.33.0 sparse-index-blame ----------------------------------------------------------------------------------- 2000.62: git blame f2/f4/a (full-v3) 0.31(0.21+0.09) 0.32(0.21+0.10) +3.2% 2000.63: git blame f2/f4/a (full-v4) 0.29(0.19+0.08) 0.31(0.21+0.08) +6.9% 2000.64: git blame f2/f4/a (sparse-v3) 0.55(0.38+0.14) 0.23(0.14+0.07) -58.2% 2000.65: git blame f2/f4/a (sparse-v4) 0.57(0.40+0.16) 0.23(0.15+0.07) -59.6% 2000.66: git blame f2/f4/f3/a (full-v3) 0.77(0.56+0.20) 0.85(0.60+0.22) +10.4% 2000.67: git blame f2/f4/f3/a (full-v4) 0.78(0.56+0.20) 0.81(0.59+0.21) +3.8% 2000.68: git blame f2/f4/f3/a (sparse-v3) 1.07(0.78+0.28) 0.72(0.52+0.19) -32.7% 2000.99: git blame f2/f4/f3/a (sparse-v4) 1.05(0.78+0.26) 0.73(0.51+0.19) -30.5% ``` We do not include paths outside the sparse checkout cone because blame currently does not support blaming files outside of the sparse definition. Attempting to do so fails with the following error: fatal: no such path '<path outside sparse definition>' in HEAD Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>

derrickstolee

I re-ran the Scalar functional tests because it's likely you hit a known FS Monitor flake.

builtin/blame.c

t/t1092-sparse-checkout-compatibility.sh

Making the 'blame with pathspec outside sparse definition' test more robust. Previously, this test just looked for a generic "failure" and didn't verify that sparse index and non-sparse index provide the same error. We now fail only when the error messages from sparse index and non-sparse index don't match (and still document the behavior in the test). Signed-off-by: Lessley Dennington <lessleydennington@gmail.com>

vdye

Nice work - especially the extra-thorough performance testing!

blame: enable and test the sparse index

ldennington requested a review from derrickstolee September 22, 2021 22:16

ldennington force-pushed the sparse-index-blame branch from 32ab4a9 to 375b924 Compare September 23, 2021 20:53

ldennington requested a review from vdye September 23, 2021 20:55

ldennington self-assigned this Sep 23, 2021

ldennington marked this pull request as ready for review September 23, 2021 20:55

ldennington force-pushed the sparse-index-blame branch from 375b924 to 6843315 Compare September 23, 2021 21:16

derrickstolee approved these changes Sep 24, 2021

View reviewed changes

t/perf/p2000-sparse-operations.sh Outdated Show resolved Hide resolved

t/perf/p2000-sparse-operations.sh Outdated Show resolved Hide resolved

t/perf/p2000-sparse-operations.sh Outdated Show resolved Hide resolved

builtin/blame.c Show resolved Hide resolved

vdye reviewed Sep 24, 2021

View reviewed changes

t/t1092-sparse-checkout-compatibility.sh Show resolved Hide resolved

ldennington force-pushed the sparse-index-blame branch 2 times, most recently from dbe21fd to c6dca27 Compare September 24, 2021 22:07

ldennington force-pushed the sparse-index-blame branch from c6dca27 to 058b7a9 Compare September 24, 2021 22:09

derrickstolee approved these changes Sep 25, 2021

View reviewed changes

builtin/blame.c Show resolved Hide resolved

ldennington force-pushed the sparse-index-blame branch from b9e1150 to 8e4980f Compare September 27, 2021 05:54

derrickstolee requested changes Sep 27, 2021

View reviewed changes

t/t1092-sparse-checkout-compatibility.sh Outdated Show resolved Hide resolved

t/t1092-sparse-checkout-compatibility.sh Outdated Show resolved Hide resolved

t/t1092-sparse-checkout-compatibility.sh Show resolved Hide resolved

ldennington force-pushed the sparse-index-blame branch 4 times, most recently from c10d236 to fb66670 Compare September 27, 2021 16:47

ldennington force-pushed the sparse-index-blame branch from fb66670 to dd14948 Compare September 27, 2021 17:07

vdye approved these changes Sep 27, 2021

View reviewed changes

derrickstolee approved these changes Sep 27, 2021

View reviewed changes

ldennington merged commit 3011dc5 into microsoft:vfs-2.33.0 Sep 27, 2021

ldennington mentioned this pull request Oct 5, 2021

Sparse Index: diff and blame builtins gitgitgadget/git#1050

Closed

dscho pushed a commit that referenced this pull request Oct 30, 2021

Merge pull request #431 from ldennington/sparse-index-blame

812b27e

blame: enable and test the sparse index

derrickstolee pushed a commit that referenced this pull request Oct 30, 2021

Merge pull request #431 from ldennington/sparse-index-blame

2acedfd

blame: enable and test the sparse index

derrickstolee pushed a commit that referenced this pull request Oct 31, 2021

Merge pull request #431 from ldennington/sparse-index-blame

0f4d7c8

blame: enable and test the sparse index

derrickstolee pushed a commit that referenced this pull request Nov 4, 2021

Merge pull request #431 from ldennington/sparse-index-blame

0e6d575

blame: enable and test the sparse index

derrickstolee pushed a commit that referenced this pull request Nov 4, 2021

Merge pull request #431 from ldennington/sparse-index-blame

cc70255

blame: enable and test the sparse index

derrickstolee pushed a commit that referenced this pull request Nov 10, 2021

Merge pull request #431 from ldennington/sparse-index-blame

1eb3f90

blame: enable and test the sparse index

derrickstolee pushed a commit that referenced this pull request Nov 15, 2021

Merge pull request #431 from ldennington/sparse-index-blame

0b5dc77

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 12, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

1034f4d

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 19, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

ac4f26c

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 20, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

1ad0dcf

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 20, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

1f6151d

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 20, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

f1706e2

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 20, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

5912b50

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 20, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

b896496

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 20, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

3a7c5c0

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 25, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

172a133

blame: enable and test the sparse index

ldennington added a commit to ldennington/git that referenced this pull request Jan 25, 2022

Merge pull request microsoft#431 from ldennington/sparse-index-blame

96c16f2

blame: enable and test the sparse index

dscho pushed a commit that referenced this pull request Feb 1, 2022

Merge pull request #431 from ldennington/sparse-index-blame

aeac8d6

blame: enable and test the sparse index

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

blame: enable and test the sparse index #431

blame: enable and test the sparse index #431

ldennington commented Sep 22, 2021 •

edited

Loading

derrickstolee commented Sep 23, 2021 •

edited

Loading

ldennington commented Sep 23, 2021

ldennington commented Sep 23, 2021

ldennington commented Sep 23, 2021

derrickstolee left a comment

derrickstolee left a comment

vdye left a comment

blame: enable and test the sparse index #431

blame: enable and test the sparse index #431

Conversation

ldennington commented Sep 22, 2021 • edited Loading

derrickstolee commented Sep 23, 2021 • edited Loading

ldennington commented Sep 23, 2021

ldennington commented Sep 23, 2021

ldennington commented Sep 23, 2021

derrickstolee left a comment

Choose a reason for hiding this comment

derrickstolee left a comment

Choose a reason for hiding this comment

vdye left a comment

Choose a reason for hiding this comment

ldennington commented Sep 22, 2021 •

edited

Loading

derrickstolee commented Sep 23, 2021 •

edited

Loading