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.

pjbgf added 2 commits December 5, 2022 19:39
- 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 force-pushed the fix-go-git-data-race branch from b9037d8 to 38b02ce Compare December 16, 2022 14:26
@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
Development

Successfully merging this pull request may close these issues.

2 participants