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

some basic splitstore refactors #7999

Merged
merged 7 commits into from
Jan 28, 2022
Merged

some basic splitstore refactors #7999

merged 7 commits into from
Jan 28, 2022

Conversation

vyzo
Copy link
Contributor

@vyzo vyzo commented Jan 25, 2022

Preliminaries for implementing additional functionality on top (sortless compaction, gc, etc), this performs some basic refactoring to avoid polluting ballooning subsequent prs:

  • The distinction between MarkSet and MarkSetVisitor is removed.
  • SetConcurrent is removed from MarkSet interface; all mark sets must be concurrent, because ...
  • Chain walking is now parallelized

@vyzo vyzo requested review from magik6k and arajasek January 25, 2022 15:33
@vyzo vyzo requested a review from a team as a code owner January 25, 2022 15:33
@codecov
Copy link

codecov bot commented Jan 25, 2022

Codecov Report

Merging #7999 (f07ce29) into master (a246865) will decrease coverage by 0.09%.
The diff coverage is 74.71%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #7999      +/-   ##
==========================================
- Coverage   39.19%   39.10%   -0.10%     
==========================================
  Files         660      660              
  Lines       71411    71428      +17     
==========================================
- Hits        27993    27932      -61     
- Misses      38598    38684      +86     
+ Partials     4820     4812       -8     
Impacted Files Coverage Δ
blockstore/splitstore/markset.go 75.00% <ø> (ø)
blockstore/splitstore/splitstore.go 27.09% <ø> (+0.21%) ⬆️
blockstore/splitstore/splitstore_check.go 0.00% <0.00%> (ø)
blockstore/splitstore/splitstore_compact.go 52.43% <77.77%> (+0.67%) ⬆️
blockstore/splitstore/splitstore_warmup.go 52.63% <84.61%> (+1.24%) ⬆️
blockstore/splitstore/markset_badger.go 68.07% <100.00%> (-1.52%) ⬇️
blockstore/splitstore/markset_map.go 83.78% <100.00%> (+2.53%) ⬆️
blockstore/splitstore/visitor.go 100.00% <100.00%> (ø)
itests/kit/blockminer.go 74.11% <0.00%> (-17.06%) ⬇️
markets/loggers/loggers.go 88.88% <0.00%> (-11.12%) ⬇️
... and 26 more

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 a246865...f07ce29. Read the comment docs.

Copy link
Contributor

@magik6k magik6k left a comment

Choose a reason for hiding this comment

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

Don't see anything obviously wrong, just one tiny nitpick

Comment on lines 707 to 708
walking := toWalk
toWalk = nil
Copy link
Contributor

Choose a reason for hiding this comment

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

Technically we don't need this copy; walking could be set to nil after populating workch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes, and we can reuse the slice; let me fix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hrm, we can't actually reuse the slice, it first comes from the tipset; will just setr to nil after copying.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done.

Copy link
Contributor Author

@vyzo vyzo Jan 26, 2022

Choose a reason for hiding this comment

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

you know, we can first make a new one and copy when we initialize and then reuse; that will save some allocations.
let me do.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done done.

@vyzo vyzo merged commit ff10e0e into master Jan 28, 2022
@vyzo vyzo deleted the feat/splistore-refactors branch January 28, 2022 11:55
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