8000 [v6-exp] plumbing: packfile, Refactor Parser and Scanner logic by pjbgf · Pull Request #1228 · go-git/go-git · GitHub
[go: up one dir, main page]

Skip to content

[v6-exp] plumbing: packfile, Refactor Parser and Scanner logic #1228

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Dec 17, 2024

Conversation

pjbgf
Copy link
Member
@pjbgf pjbgf commented Nov 17, 2024

The changes focus on increasing thread-safety, simplifying the code base,
enabling support for sha256 and improving general time and space complexities.

Raw allocations for a packfile parse dropped on average 39%:

                                                                │ /tmp/before  │              /tmp/after               │
                                                                │  allocs/op   │  allocs/op   vs base                  │
Parse/https://github.com/git-fixtures/root-references.git-16       1.987k ± 0%   1.191k ± 0%  -40.06% (p=0.000 n=10)
Parse/https://github.com/git-fixtures/basic.git-16                 1034.0 ± 0%    613.0 ± 0%  -40.72% (p=0.000 n=10)
Parse/https://github.com/git-fixtures/basic.git#01-16               941.0 ± 0%    576.0 ± 0%  -38.79% (p=0.000 n=10)
Parse/https://github.com/git-fixtures/basic.git#02-16               880.0 ± 0%    547.0 ± 0%  -37.84% (p=0.000 n=10)
Parse/https://github.com/src-d/go-git.git-16                      124.06k ± 0%   67.12k ± 0%  -45.90% (p=0.000 n=10)
Parse/https://github.com/git-fixtures/tags.git-16                   212.0 ± 0%    155.0 ± 0%  -26.89% (p=0.000 n=10)
Parse/https://github.com/spinnaker/spinnaker.git-16                195.6k ± 0%   106.8k ± 0%  -45.41% (p=0.000 n=10)
Parse/https://github.com/jamesob/desk.git-16                       22.52k ± 0%   12.20k ± 0%  -45.82% (p=0.000 n=10)
Parse/https://github.com/cpcs499/Final_Pres_P.git-16                65.00 ± 0%    68.00 ± 0%   +4.62% (p=0.000 n=10)
Parse/https://github.com/github/gem-builder.git-16                 3.237k ± 0%   1.778k ± 0%  -45.07% (p=0.000 n=10)
Parse/https://github.com/githubtraining/example-branches.git-16     871.0 ± 0%    529.0 ± 0%  -39.27% (p=0.000 n=10)
Parse/https://github.com/rumpkernel/rumprun-xen.git-16            127.86k ± 0%   70.23k ± 0%  -45.07% (p=0.000 n=10)
Parse/https://github.com/mcuadros/skeetr.git-16                    9.334k ± 0%   5.317k ± 0%  -43.04% (p=0.000 n=10)
Parse/https://github.com/dezfowler/LiteMock.git-16                 1.892k ± 0%   1.134k ± 0%  -40.06% (p=0.000 n=10)
Parse/https://github.com/tyba/storable.git-16                      47.22k ± 0%   25.34k ± 0%  -46.32% (p=0.000 n=10)
Parse/https://github.com/toqueteos/ts3.git-16                      4.246k ± 0%   2.438k ± 0%  -42.58% (p=0.000 n=10)

GetByOffset calls no longer create new allocations for cached operations:

                               │ /tmp/before  │               /tmp/after                │
                               │     B/op     │    B/op     vs base                     │
GetByOffset/with_storage-16      6.052µ ± 3%   3.887µ ± 2%  -35.78% (p=0.000 n=10)
                               │     B/op     │    B/op     vs base                     │
GetByOffset/with_storage-16      384.0 ± 0%       0.0 ± 0%  -100.00% (p=0.000 n=10)
                               │  allocs/op   │ allocs/op   vs base                     │
GetByOffset/with_storage-16      4.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)

The test files modified as part of this commit started replacing the check.v1 test framework with stretchr/testify. That shall continue as other changes take place.

Breaking Changes

  • Removal of the concept of Large Object Threshold.
  • Rename packfile.ObjectHeader from Length to Size.
  • Both Parser and Scanner APIs have changed. They now rely on the Functional Options pattern for increased extensibility.

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
The changes focus on increasing thread-safety, simplifying the code base,
enabling support for sha256 and improving general time and space complexities.

Raw allocations for a packfile parse dropped on average 39%:

                                                                │ /tmp/before  │              /tmp/after               │
                                                                │  allocs/op   │  allocs/op   vs base                  │
Parse/https://github.com/git-fixtures/root-references.git-16       1.987k ± 0%   1.191k ± 0%  -40.06% (p=0.000 n=10)
Parse/https://github.com/git-fixtures/basic.git-16                 1034.0 ± 0%    613.0 ± 0%  -40.72% (p=0.000 n=10)
Parse/https://github.com/git-fixtures/basic.git#01-16               941.0 ± 0%    576.0 ± 0%  -38.79% (p=0.000 n=10)
Parse/https://github.com/git-fixtures/basic.git#02-16               880.0 ± 0%    547.0 ± 0%  -37.84% (p=0.000 n=10)
Parse/https://github.com/src-d/go-git.git-16                      124.06k ± 0%   67.12k ± 0%  -45.90% (p=0.000 n=10)
Parse/https://github.com/git-fixtures/tags.git-16                   212.0 ± 0%    155.0 ± 0%  -26.89% (p=0.000 n=10)
Parse/https://github.com/spinnaker/spinnaker.git-16                195.6k ± 0%   106.8k ± 0%  -45.41% (p=0.000 n=10)
Parse/https://github.com/jamesob/desk.git-16                       22.52k ± 0%   12.20k ± 0%  -45.82% (p=0.000 n=10)
Parse/https://github.com/cpcs499/Final_Pres_P.git-16                65.00 ± 0%    68.00 ± 0%   +4.62% (p=0.000 n=10)
Parse/https://github.com/github/gem-builder.git-16                 3.237k ± 0%   1.778k ± 0%  -45.07% (p=0.000 n=10)
Parse/https://github.com/githubtraining/example-branches.git-16     871.0 ± 0%    529.0 ± 0%  -39.27% (p=0.000 n=10)
Parse/https://github.com/rumpkernel/rumprun-xen.git-16            127.86k ± 0%   70.23k ± 0%  -45.07% (p=0.000 n=10)
Parse/https://github.com/mcuadros/skeetr.git-16                    9.334k ± 0%   5.317k ± 0%  -43.04% (p=0.000 n=10)
Parse/https://github.com/dezfowler/LiteMock.git-16                 1.892k ± 0%   1.134k ± 0%  -40.06% (p=0.000 n=10)
Parse/https://github.com/tyba/storable.git-16                      47.22k ± 0%   25.34k ± 0%  -46.32% (p=0.000 n=10)
Parse/https://github.com/toqueteos/ts3.git-16                      4.246k ± 0%   2.438k ± 0%  -42.58% (p=0.000 n=10)

GetByOffset calls no longer create new allocations for cached operations:

                               │ /tmp/before  │               /tmp/after                │
                               │     B/op     │    B/op     vs base                     │
GetByOffset/with_storage-16      6.052µ ± 3%   3.887µ ± 2%  -35.78% (p=0.000 n=10)
                               │     B/op     │    B/op     vs base                     │
GetByOffset/with_storage-16      384.0 ± 0%       0.0 ± 0%  -100.00% (p=0.000 n=10)
                               │  allocs/op   │ allocs/op   vs base                     │
GetByOffset/with_storage-16      4.000 ± 0%     0.000 ± 0%  -100.00% (p=0.000 n=10)

The test files modified as part of this commit started replacing the check.v1
test framework with stretchr/testify. That shall continue as other changes
take place.

Breaking Changes
- Removal of the concept of Large Object Threshold.
- Rename packfile.ObjectHeader from Length to Size.
- Both Parser and Scanner APIs have changed. They now rely on the
Functional Options pattern for increased extensibility.

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Since the multi_ack implementation (go-git#1204), Azure DevOps works
out of the box, no longer requiring code changes. Therefore,
the previous devops specific example is no longer needed.

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
@pjbgf pjbgf added the breaking label Nov 17, 2024
@pjbgf pjbgf added this to the v6.0.0 milestone Nov 17, 2024
pjbgf added 5 commits December 3, 2024 12:21
Add arguments to the seed corpus, to improve overall fuzzing reachability.
Some additional tests were also added for patch delta.

The revision now limits its parsing to 128kb.

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
@pjbgf pjbgf changed the title plumbing: packfile, Refactor Parser and Scanner logic [v6-exp] plumbing: packfile, Refactor Parser and Scanner logic Dec 15, 2024
Based on the current settings on oss-fuzz, it is quite likely that any changes
to fuzzing or adjancent files may cause the fuzzing the break, as the build
script used is for the master branch. Once that logic changes, this can be
enabled again for v6-exp.

For more information:
https://github.com/google/oss-fuzz/blob/master/projects/go-git/Dockerfile#L18

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
@pjbgf pjbgf merged commit dfe805c into go-git:v6-exp Dec 17, 2024
12 checks passed
@pjbgf pjbgf deleted the v6-pack branch December 17, 2024 19:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant
0