search

dragonflyoss / nydus

Nydus - the Dragonfly image service, providing fast, secure and easy access to container images.
https://nydus.dev/
Apache License 2.0
1.23k stars 205 forks source link

refactor: fixed the unnecessary mutex lock operation #1616

Closed Chasing1020 closed 2 months ago

Chasing1020 commented 2 months ago

Relevant Issue (if applicable)

If there are Issues related to this PullRequest, please list it.

Details

This PR is a preliminary proposal, and I welcome friendly discussions on this issue.

Due to Rc is neither Send or Sync, it cannot be sent between threads safely. Rust cannot send an Rc to another thread, then you have to construct the value on the thread that will use it, or send the value to that thread before wrapping it in an Rc. So rustc's mechanism will not exploit the Mutex's protection.

IMHO, for the most part, we should use Rc<RefCell<T>> for single thread or Arc<Mutex<T>> for exclusive access (until compiler complain about our code need Send or Sync).

Thank you for your time!

Types of changes

What types of changes does your PullRequest introduce? Put an x in all the boxes that apply:

Checklist

Go over all the following points, and put an x in all the boxes that apply.

codecov[bot] commented 2 months ago

Codecov Report

Attention: Patch coverage is 78.12500% with 14 lines in your changes missing coverage. Please review.

Project coverage is 61.28%. Comparing base (d89410f) to head (85ae482). Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
builder/src/directory.rs 0.00% 3 Missing :warning:
builder/src/chunkdict_generator.rs 0.00% 2 Missing :warning:
builder/src/core/prefetch.rs 85.71% 2 Missing :warning:
builder/src/compact.rs 66.66% 1 Missing :warning:
builder/src/core/blob.rs 0.00% 1 Missing :warning:
builder/src/core/layout.rs 66.66% 0 Missing and 1 partial :warning:
builder/src/core/v6.rs 75.00% 0 Missing and 1 partial :warning:
src/bin/nydus-image/deduplicate.rs 0.00% 1 Missing :warning:
src/bin/nydus-image/stat.rs 0.00% 1 Missing :warning:
src/bin/nydus-image/validator.rs 0.00% 1 Missing :warning:
Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616/graphs/tree.svg?width=650&height=150&src=pr&token=OBUY0LM4EN&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss)](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) ```diff @@ Coverage Diff @@ ## master #1616 +/- ## ========================================== - Coverage 61.29% 61.28% -0.01% ========================================== Files 146 146 Lines 48143 48143 Branches 46110 46110 ========================================== - Hits 29510 29506 -4 - Misses 17074 17075 +1 - Partials 1559 1562 +3 ``` | [Files with missing lines](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) | Coverage Δ | | |---|---|---| | [builder/src/core/bootstrap.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Fcore%2Fbootstrap.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvY29yZS9ib290c3RyYXAucnM=) | `77.33% <100.00%> (ø)` | | | [builder/src/core/tree.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Fcore%2Ftree.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvY29yZS90cmVlLnJz) | `83.86% <100.00%> (-0.29%)` | :arrow_down: | | [builder/src/core/v5.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Fcore%2Fv5.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvY29yZS92NS5ycw==) | `76.64% <100.00%> (ø)` | | | [builder/src/merge.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Fmerge.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvbWVyZ2UucnM=) | `70.79% <100.00%> (ø)` | | | [builder/src/stargz.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Fstargz.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvc3Rhcmd6LnJz) | `74.67% <100.00%> (ø)` | | | [builder/src/tarball.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Ftarball.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvdGFyYmFsbC5ycw==) | `62.84% <100.00%> (ø)` | | | [builder/src/compact.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Fcompact.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvY29tcGFjdC5ycw==) | `80.32% <66.66%> (ø)` | | | [builder/src/core/blob.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Fcore%2Fblob.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvY29yZS9ibG9iLnJz) | `41.33% <0.00%> (ø)` | | | [builder/src/core/layout.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Fcore%2Flayout.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvY29yZS9sYXlvdXQucnM=) | `92.10% <66.66%> (ø)` | | | [builder/src/core/v6.rs](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree&filepath=builder%2Fsrc%2Fcore%2Fv6.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss#diff-YnVpbGRlci9zcmMvY29yZS92Ni5ycw==) | `75.51% <75.00%> (ø)` | | | ... and [6 more](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss) | | ... and [1 file with indirect coverage changes](https://app.codecov.io/gh/dragonflyoss/nydus/pull/1616/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dragonflyoss)