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

Dummy sync implementation for no_std #1008

Merged

Conversation

devrandom
Copy link
Member

We would like to not bother synchronizing access on no_std platforms, because there is no mature rust multi-threading support for them yet. Accordingly, this PR implements drop-in replacements for Mutex, Condvar and (not done yet) RwLock.

See also discussion in #842 (comment) - but this PR uses a slightly different approach.

in preparation for no-std sync dummies
@codecov
Copy link

codecov bot commented Jul 19, 2021

Codecov Report

Merging #1008 (2e8f4fe) into main (1f1d7c6) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1008      +/-   ##
==========================================
- Coverage   90.76%   90.76%   -0.01%     
==========================================
  Files          60       60              
  Lines       30909    30909              
==========================================
- Hits        28056    28055       -1     
- Misses       2853     2854       +1     
Impacted Files Coverage Δ
lightning/src/chain/chainmonitor.rs 95.80% <ø> (ø)
lightning/src/chain/channelmonitor.rs 90.80% <ø> (ø)
lightning/src/ln/chanmon_update_fail_tests.rs 97.94% <ø> (ø)
lightning/src/ln/channelmanager.rs 84.85% <ø> (ø)
lightning/src/ln/functional_test_utils.rs 94.87% <ø> (ø)
lightning/src/ln/functional_tests.rs 97.34% <ø> (-0.02%) ⬇️
lightning/src/ln/peer_handler.rs 46.44% <ø> (ø)
lightning/src/routing/network_graph.rs 91.81% <ø> (ø)
lightning/src/util/enforcing_trait_impls.rs 90.38% <ø> (ø)
lightning/src/util/logger.rs 82.14% <ø> (ø)
... and 7 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 1f1d7c6...2e8f4fe. Read the comment docs.

Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

Cool! Glad it kinda just worked. CI should also at least run cargo test --no-default-features --features no_std (it currently just runs with hashbrown explicitly), but that seems to pass locally.

lightning/src/sync.rs Outdated Show resolved Hide resolved
@devrandom
Copy link
Member Author

Cool! Glad it kinda just worked. CI should also at least run cargo test --no-default-features --features no_std (it currently just runs with hashbrown explicitly), but that seems to pass locally.

I read what you said as "instead of --features hashbrown", but let me know if I misunderstood. Will do.

@devrandom devrandom marked this pull request as ready for review July 20, 2021 10:34
@devrandom devrandom requested a review from jkczyz July 20, 2021 15:05
lightning/src/lib.rs Show resolved Hide resolved
lightning/src/ln/channel.rs Outdated Show resolved Hide resolved
lightning/src/lib.rs Outdated Show resolved Hide resolved
lightning/src/sync.rs Show resolved Hide resolved
lightning/src/sync.rs Outdated Show resolved Hide resolved
@devrandom devrandom force-pushed the 2021-07-sync-no-std branch 2 times, most recently from 7e6557d to 50f4847 Compare July 20, 2021 18:49
Copy link
Contributor

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

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

ACK 2e8f4fe

@TheBlueMatt TheBlueMatt merged commit 41d8d4d into lightningdevkit:main Jul 22, 2021
@devrandom devrandom deleted the 2021-07-sync-no-std branch July 31, 2021 19:40
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

3 participants