Skip to content
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

chore: Fix race in go progress #198

Merged
merged 3 commits into from
Sep 11, 2023
Merged

Conversation

willmurphyscode
Copy link
Contributor

Previously, setting Current on the stager was technically a race condition, since the reference was shared between go routines. go-progress added an atomic version of stager that atomically updates the current stage (wagoodman/go-progress#2). Upgrade to use that stager implementation, and pass -race to the unit test step to help prevent new race conditions from being introduced.

The next step will be to merge this into syft, see comparison: https://github.com/anchore/syft/compare/fix-go-progress-race

Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
Signed-off-by: Will Murphy <will.murphy@anchore.com>
@github-actions
Copy link

Benchmark Test Results

Benchmark results from the latest changes vs base branch
latest: Pulling from library/ubuntu
tar: Option --mtime: Treating date 'UTC 2019-09-16' as 2019-09-16 00:00:00
goos: linux
goarch: amd64
pkg: github.com/anchore/stereoscope/pkg/file
cpu: Intel(R) Xeon(R) Platinum 8370C CPU @ 2.80GHz
docker: 
           │ ./.tmp/benchmark-e08f009.txt │
           │            sec/op            │
TarIndex-2                   44.18µ ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

           │ ./.tmp/benchmark-e08f009.txt │
           │             B/op             │
TarIndex-2                  5.560Ki ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

           │ ./.tmp/benchmark-e08f009.txt │
           │          allocs/op           │
TarIndex-2                    93.00 ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

pkg: github.com/anchore/stereoscope/test/integration
                                      │ ./.tmp/benchmark-e08f009.txt │
                                      │            sec/op            │
SimpleImage_GetImage/docker-archive-2                   1.440m ± ∞ ¹
SimpleImage_GetImage/oci-archive-2                      1.437m ± ∞ ¹
SimpleImage_GetImage/oci-dir-2                          1.129m ± ∞ ¹
geomean                                                 1.327m
¹ need >= 6 samples for confidence interval at level 0.95

                                      │ ./.tmp/benchmark-e08f009.txt │
                                      │             B/op             │
SimpleImage_GetImage/docker-archive-2                  328.0Ki ± ∞ ¹
SimpleImage_GetImage/oci-archive-2                     645.7Ki ± ∞ ¹
SimpleImage_GetImage/oci-dir-2                         413.0Ki ± ∞ ¹
geomean                                                443.9Ki
¹ need >= 6 samples for confidence interval at level 0.95

                                      │ ./.tmp/benchmark-e08f009.txt │
                                      │          allocs/op           │
SimpleImage_GetImage/docker-archive-2                   2.736k ± ∞ ¹
SimpleImage_GetImage/oci-archive-2                      1.570k ± ∞ ¹
SimpleImage_GetImage/oci-dir-2                          1.354k ± ∞ ¹
geomean                                                 1.798k
¹ need >= 6 samples for confidence interval at level 0.95

docker: Error response from daemon: Get "http://localhost/v2/": dial tcp [::1]:80: connect: connection refused.
                                                   │ ./.tmp/benchmark-e08f009.txt │
                                                   │            sec/op            │
SimpleImage_FetchSquashedContents/docker-archive-2                   17.60µ ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

                                                   │ ./.tmp/benchmark-e08f009.txt │
                                                   │             B/op             │
SimpleImage_FetchSquashedContents/docker-archive-2                  2.648Ki ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

                                                   │ ./.tmp/benchmark-e08f009.txt │
                                                   │          allocs/op           │
SimpleImage_FetchSquashedContents/docker-archive-2                    21.00 ± ∞ ¹
¹ need >= 6 samples for confidence interval at level 0.95

@willmurphyscode willmurphyscode merged commit 2fc2d6c into main Sep 11, 2023
@willmurphyscode willmurphyscode deleted the fix-race-in-go-progress branch September 11, 2023 19:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants