Skip to content
This repository has been archived by the owner on Sep 14, 2023. It is now read-only.

Fix data race when running go-git tests #1

Merged
merged 4 commits into from Dec 16, 2022

Conversation

pjbgf
Copy link
Member

@pjbgf pjbgf commented Dec 16, 2022

Apart from the main change highlighted in the title, this PR also:

  • build: Update GitHub workflows.
    • Remove unecessary token permissions in CI.
    • Bump Action versions to latest.
    • Bump Go version to match go-git.
  • build: Bump dependencies.
  • Fixes current broken tests when running on Windows.

The go-git performance impact whilst running the Parse benchmarks are subtle:

name                                                             old time/op    new time/op    delta
Parse/https://github.com/git-fixtures/root-references.git-16       2.40ms ± 6%    2.37ms ± 1%   ~     (p=0.700 n=3+3)
Parse/https://github.com/git-fixtures/basic.git-16                 1.95ms ± 0%    1.96ms ± 0%   ~     (p=0.200 n=3+3)
Parse/https://github.com/git-fixtures/basic.git#01-16              1.97ms ± 0%    2.01ms ± 1%   ~     (p=0.100 n=3+3)
Parse/https://github.com/git-fixtures/basic.git#02-16              1.94ms ± 0%    2.00ms ± 1%   ~     (p=0.100 n=3+3)
Parse/https://github.com/src-d/go-git.git-16                        391ms ± 2%     402ms ± 1%   ~     (p=0.200 n=3+3)
Parse/https://github.com/git-fixtures/tags.git-16                  65.0µs ± 1%    66.9µs ± 1%   ~     (p=0.100 n=3+3)
Parse/https://github.com/spinnaker/spinnaker.git-16                 104ms ± 0%     110ms ± 2%   ~     (p=0.100 n=3+3)
Parse/https://github.com/jamesob/desk.git-16                       14.1ms ± 2%    14.4ms ± 1%   ~     (p=0.100 n=3+3)
Parse/https://github.com/cpcs499/Final_Pres_P.git-16               18.0µs ± 1%    18.2µs ± 3%   ~     (p=1.000 n=3+3)
Parse/https://github.com/github/gem-builder.git-16                 1.25ms ± 4%    1.22ms ± 1%   ~     (p=0.700 n=3+3)
Parse/https://github.com/githubtraining/example-branches.git-16     257µs ± 2%     258µs ± 1%   ~     (p=1.000 n=3+3)
Parse/https://github.com/rumpkernel/rumprun-xen.git-16              120ms ± 3%     117ms ± 1%   ~     (p=0.400 n=3+3)
Parse/https://github.com/mcuadros/skeetr.git-16                    3.42ms ± 1%    3.44ms ± 2%   ~     (p=1.000 n=3+3)
Parse/https://github.com/dezfowler/LiteMock.git-16                 6.02ms ± 2%    6.07ms ± 1%   ~     (p=0.700 n=3+3)
Parse/https://github.com/tyba/storable.git-16                      18.4ms ± 3%    18.1ms ± 1%   ~     (p=0.700 n=3+3)
Parse/https://github.com/toqueteos/ts3.git-16                      1.74ms ± 1%    1.71ms ± 1%   ~     (p=0.100 n=3+3)
Parse/#00-16                                                        393µs ± 1%     370µs ± 0%   ~     (p=0.100 n=3+3)
Parse/#01-16                                                       2.72ms ± 5%    2.62ms ± 2%   ~     (p=0.200 n=3+3)
ParseBasic-16                                                      2.09ms ± 3%    2.06ms ± 0%   ~     (p=0.200 n=3+3)
Parser-16                                                          9.01ms ± 3%    8.74ms ± 2%   ~     (p=0.100 n=3+3)

name                                                             old alloc/op   new alloc/op   delta
Parse/https://github.com/git-fixtures/root-references.git-16        474kB ± 0%     473kB ± 1%   ~     (p=1.000 n=3+3)
Parse/https://github.com/git-fixtures/basic.git-16                  247kB ± 1%     247kB ± 1%   ~     (p=1.000 n=3+3)
Parse/https://github.com/git-fixtures/basic.git#01-16               245kB ± 1%     246kB ± 1%   ~     (p=0.400 n=3+3)
Parse/https://github.com/git-fixtures/basic.git#02-16               222kB ± 1%     221kB ± 1%   ~     (p=0.400 n=3+3)
Parse/https://github.com/src-d/go-git.git-16                       37.9MB ± 1%    39.9MB ± 5%   ~     (p=0.200 n=3+3)
Parse/https://github.com/git-fixtures/tags.git-16                  87.5kB ± 0%    87.5kB ± 0%   ~     (p=0.700 n=3+3)
Parse/https://github.com/spinnaker/spinnaker.git-16                44.2MB ± 0%    44.3MB ± 0%   ~     (p=0.200 n=3+3)
Parse/https://github.com/jamesob/desk.git-16                       4.59MB ± 0%    4.59MB ± 0%   ~     (p=1.000 n=3+3)
Parse/https://github.com/cpcs499/Final_Pres_P.git-16               16.9kB ± 0%    16.9kB ± 0%   ~     (p=0.600 n=3+3)
Parse/https://github.com/github/gem-builder.git-16                  649kB ± 0%     649kB ± 0%   ~     (p=1.000 n=3+3)
Parse/https://github.com/githubtraining/example-branches.git-16     210kB ± 0%     210kB ± 0%   ~     (p=1.000 n=3+3)
Parse/https://github.com/rumpkernel/rumprun-xen.git-16             33.7MB ± 2%    33.0MB ± 2%   ~     (p=0.400 n=3+3)
Parse/https://github.com/mcuadros/skeetr.git-16                    1.92MB ± 0%    1.92MB ± 0%   ~     (p=1.000 n=3+3)
Parse/https://github.com/dezfowler/LiteMock.git-16                  425kB ± 0%     425kB ± 1%   ~     (p=1.000 n=3+3)
Parse/https://github.com/tyba/storable.git-16                      10.3MB ± 0%    10.2MB ± 0%   ~     (p=0.700 n=3+3)
Parse/https://github.com/toqueteos/ts3.git-16                       777kB ± 0%     777kB ± 0%   ~     (p=0.700 n=3+3)
Parse/#00-16                                                        329kB ± 0%     329kB ± 0%   ~     (p=0.100 n=3+3)
Parse/#01-16                                                       1.01MB ± 0%    1.01MB ± 0%   ~     (p=1.000 n=3+3)
ParseBasic-16                                                       236kB ± 0%     236kB ± 1%   ~     (p=0.700 n=3+3)
Parser-16                                                          2.15MB ± 1%    2.13MB ± 1%   ~     (p=0.400 n=3+3)

name                                                             old allocs/op  new allocs/op  delta
Parse/https://github.com/git-fixtures/root-references.git-16        1.36k ± 0%     1.36k ± 0%   ~     (all equal)
Parse/https://github.com/git-fixtures/basic.git-16                    704 ± 0%       704 ± 0%   ~     (all equal)
Parse/https://github.com/git-fixtures/basic.git#01-16                 688 ± 0%       688 ± 0%   ~     (all equal)
Parse/https://github.com/git-fixtures/basic.git#02-16                 621 ± 0%       620 ± 0%   ~     (p=0.400 n=3+3)
Parse/https://github.com/src-d/go-git.git-16                        78.1k ± 0%     78.1k ± 0%   ~     (p=0.500 n=3+3)
Parse/https://github.com/git-fixtures/tags.git-16                     164 ± 0%       164 ± 0%   ~     (all equal)
Parse/https://github.com/spinnaker/spinnaker.git-16                  115k ± 0%      115k ± 0%   ~     (p=0.100 n=3+3)
Parse/https://github.com/jamesob/desk.git-16                        13.1k ± 0%     13.1k ± 0%   ~     (p=1.000 n=3+3)
Parse/https://github.com/cpcs499/Final_Pres_P.git-16                 63.0 ± 0%      63.0 ± 0%   ~     (all equal)
Parse/https://github.com/github/gem-builder.git-16                  1.86k ± 0%     1.86k ± 0%   ~     (all equal)
Parse/https://github.com/githubtraining/example-branches.git-16       578 ± 0%       578 ± 0%   ~     (all equal)
Parse/https://github.com/rumpkernel/rumprun-xen.git-16              73.8k ± 0%     73.8k ± 0%   ~     (p=0.100 n=3+3)
Parse/https://github.com/mcuadros/skeetr.git-16                     5.77k ± 0%     5.77k ± 0%   ~     (p=1.000 n=3+3)
Parse/https://github.com/dezfowler/LiteMock.git-16                  1.38k ± 0%     1.38k ± 0%   ~     (p=1.000 n=3+3)
Parse/https://github.com/tyba/storable.git-16                       26.2k ± 0%     26.2k ± 0%   ~     (p=0.300 n=3+3)
Parse/https://github.com/toqueteos/ts3.git-16                       2.52k ± 0%     2.52k ± 0%   ~     (all equal)
Parse/#00-16                                                          974 ± 0%       974 ± 0%   ~     (all equal)
Parse/#01-16                                                        3.38k ± 0%     3.38k ± 0%   ~     (p=1.000 n=3+3)
ParseBasic-16                                                         702 ± 0%       702 ± 0%   ~     (all equal)
Parser-16                                                           3.49k ± 0%     3.49k ± 0%   ~     (p=0.300 n=3+3)

Same PR as upstream's: go-git#28.

- Remove unecessary token permissions in CI.
- Bump Action versions to latest.
- Bump Go version to match go-git repository.

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
memfs/storage.go Show resolved Hide resolved
memfs/storage.go Show resolved Hide resolved
memfs/storage.go Outdated
@@ -220,6 +227,7 @@ func (c *content) ReadAt(b []byte, off int64) (n int, err error) {
}

btr := c.bytes[off : off+l]
c.m.RUnlock()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't this go the end of the func? asking because, i think btr would be a pointer to a sub slice of c.bytes?

Signed-off-by: Paulo Gomes <pjbgf@linux.com>
@pjbgf pjbgf force-pushed the fix-go-git-data-race branch 2 times, most recently from f139c3c to b9037d8 Compare December 16, 2022 14:10
Signed-off-by: Paulo Gomes <pjbgf@linux.com>
@pjbgf pjbgf merged commit 392d7eb into fluxcd:master Dec 16, 2022
@pjbgf pjbgf deleted the fix-go-git-data-race branch December 19, 2022 17:50
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants