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

Fix race in gc sweep #1692

Merged
merged 1 commit into from Oct 31, 2017
Merged

Fix race in gc sweep #1692

merged 1 commit into from Oct 31, 2017

Conversation

dmcgowan
Copy link
Member

Removes extra goroutine and calls removal and scan in same thread

@codecov-io
Copy link

codecov-io commented Oct 27, 2017

Codecov Report

Merging #1692 into master will decrease coverage by 0.12%.
The diff coverage is 75%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #1692      +/-   ##
=========================================
- Coverage   49.03%   48.9%   -0.13%     
=========================================
  Files          27      27              
  Lines        4087    4071      -16     
=========================================
- Hits         2004    1991      -13     
+ Misses       1664    1662       -2     
+ Partials      419     418       -1
Impacted Files Coverage Δ
metadata/gc.go 61.96% <100%> (+0.71%) ⬆️
metadata/db.go 66.66% <57.14%> (-0.16%) ⬇️
metadata/content.go 30.62% <0%> (-0.96%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2f9fbf...4326702. Read the comment docs.

Removes extra goroutine and calls removal and scan in same thread

Signed-off-by: Derek McGowan <derek@mcgstyle.net>
@dmcgowan
Copy link
Member Author

Rebased on master with race fixes

GC benchmarks with this change

benchmark                                old ns/op     new ns/op     delta
BenchmarkGarbageCollect/10-Sets-8        761321        525422        -30.99%
BenchmarkGarbageCollect/100-Sets-8       6642323       5667259       -14.68%
BenchmarkGarbageCollect/1000-Sets-8      69455007      57987159      -16.51%
BenchmarkGarbageCollect/10000-Sets-8     818302653     733116817     -10.41%

benchmark                                old allocs     new allocs     delta
BenchmarkGarbageCollect/10-Sets-8        2485           2474           -0.44%
BenchmarkGarbageCollect/100-Sets-8       24025          24004          -0.09%
BenchmarkGarbageCollect/1000-Sets-8      258620         258589         -0.01%
BenchmarkGarbageCollect/10000-Sets-8     2805237        2805273        +0.00%

benchmark                                old bytes     new bytes     delta
BenchmarkGarbageCollect/10-Sets-8        144948        133868        -7.64%
BenchmarkGarbageCollect/100-Sets-8       1320973       1228000       -7.04%
BenchmarkGarbageCollect/1000-Sets-8      16509058      14964996      -9.35%
BenchmarkGarbageCollect/10000-Sets-8     174331384     157471128     -9.67%

@crosbymichael
Copy link
Member

LGTM

Copy link
Member

@estesp estesp left a comment

Choose a reason for hiding this comment

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

LGTM

@estesp estesp merged commit 474d4df into containerd:master Oct 31, 2017
@dmcgowan dmcgowan deleted the gc-fix-sweep-race branch June 5, 2018 18:21
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.

None yet

4 participants