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(lib/blocktree): replace sync.Map with mutex protected map #2285

Merged
merged 6 commits into from
Feb 22, 2022

Conversation

qdm12
Copy link
Contributor

@qdm12 qdm12 commented Feb 9, 2022

Changes

  • Replace sync.Map with typed mutex protected map of hash to runtime.Instance
  • Rename runtime -> runtimes in BlockTree since it's a mapping from hash to runtime instance, not just a single runtime
  • Add unit tests for map

Tests

go test ./lib/blocktree

Issues

Primary Reviewer

@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #2285 (56826d9) into development (0eacba3) will increase coverage by 0.04%.
The diff coverage is 85.18%.

Impacted file tree graph

@@               Coverage Diff               @@
##           development    #2285      +/-   ##
===============================================
+ Coverage        57.58%   57.63%   +0.04%     
===============================================
  Files              211      212       +1     
  Lines            27906    27922      +16     
===============================================
+ Hits             16070    16092      +22     
+ Misses           10207    10199       -8     
- Partials          1629     1631       +2     
Flag Coverage Δ
unit-tests 57.63% <85.18%> (+0.04%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
lib/blocktree/blocktree.go 55.95% <63.63%> (ø)
lib/blocktree/hashtoruntime.go 100.00% <100.00%> (ø)
dot/state/block_notify.go 81.90% <0.00%> (-8.58%) ⬇️
lib/runtime/wasmer/imports.go 48.71% <0.00%> (+0.05%) ⬆️
dot/network/service.go 55.88% <0.00%> (+0.45%) ⬆️
dot/network/light.go 86.05% <0.00%> (+0.79%) ⬆️
dot/peerset/peerstate.go 75.94% <0.00%> (+1.26%) ⬆️
dot/peerset/peerset.go 49.18% <0.00%> (+1.62%) ⬆️

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 d20b8c3...56826d9. Read the comment docs.

@qdm12 qdm12 changed the title chore(lib/blocktree): replace sync.Map with map+mutex chore(lib/blocktree): replace sync.Map with mutex protected map Feb 12, 2022
@qdm12 qdm12 force-pushed the qdm12-lib-blocktree-syncmap branch from abaff34 to 61fdb65 Compare February 12, 2022 14:06
"github.com/ChainSafe/gossamer/lib/runtime"
)

type hashToInstance struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
type hashToInstance struct {
type hashToRuntimeInstance struct {

Copy link
Contributor Author

@qdm12 qdm12 Feb 18, 2022

Choose a reason for hiding this comment

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

Renaming it to hashToRuntime it makes more sense I think (and also is shorter)

Copy link
Contributor

@kishansagathiya kishansagathiya left a comment

Choose a reason for hiding this comment

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

🎉

@qdm12 qdm12 force-pushed the qdm12-lib-blocktree-syncmap branch from 61fdb65 to 56826d9 Compare February 18, 2022 18:58
@qdm12 qdm12 merged commit 7d808f1 into development Feb 22, 2022
@qdm12 qdm12 deleted the qdm12-lib-blocktree-syncmap branch February 22, 2022 15:49
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.

3 participants