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.

Sign up for GitHub

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

Jump to bottom

EXPERIMENTAL DO NOT MERGE: build: replace async-lock with tokio locks in core-crypto #766

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

EXPERIMENTAL DO NOT MERGE: build: replace async-lock with tokio locks in core-crypto #766

wants to merge 1 commit into from

Conversation

coriolinus
Copy link
Contributor

This is experimental work, the purpose of which is to see how hard it would be to just swap out the lock implementations. If all had gone well, then it would have been worthwhile to make this change for the map and try_map that tokio's lock guards have, which could have improved the ergonomics of some of our internal methods.

Unfortunately, the difficulty was more than trivial. While it took only a few minutes of mechanical work to swap out the imports and uses, this causes a handful of tests to fail. So it's not really worth proceeding further with this experiment until we've actually prioritized it.

What's new in this PR

PR Submission Checklist for internal contributors
  • The PR Title
    • conforms to the style of semantic commits messages¹ supported in Wire's Github Workflow²
    • contains a reference JIRA issue number like SQPIT-764
    • answers the question: If merged, this PR will: ... ³
  1. https://sparkbox.com/foundry/semantic_commit_messages
  2. https://github.com/wireapp/.github#usage
  3. E.g. feat(conversation-list): Sort conversations by most emojis in the title #SQPIT-764.

@coriolinus
build: replace async-lock with tokio locks in core-crypto
c8998dc 
This is experimental work, the purpose of which is to see how hard
it would be to just swap out the lock implementations. If all had
gone well, then it would have been worthwhile to make this change
for the `map` and `try_map` that `tokio`'s lock guards have, which
could have improved the ergonomics of some of our internal methods.

Unfortunately, the difficulty was more than trivial. While it took
only a few minutes of mechanical work to swap out the imports and
uses, this causes a handful of tests to fail. So it's not really
worth proceeding further with this experiment until we've actually
prioritized it.
@codecov-commenter
Copy link

Codecov Report

Attention: Patch coverage is 88.88889% with 1 line in your changes missing coverage. Please review.

Project coverage is 72.05%. Comparing base (6d9b073) to head (c8998dc).

Files with missing lines Patch % Lines
crypto/src/group_store.rs 75.00% 1 Missing ⚠️
Additional details and impacted files 
@@           Coverage Diff           @@
##             main     #766   +/-   ##
=======================================
  Coverage   72.05%   72.05%           
=======================================
  Files         106      106           
  Lines       19737    19737           
=======================================
  Hits        14222    14222           
  Misses       5515     5515
Files with missing lines Coverage Δ
crypto/src/context.rs 73.46% <100.00%> (ø)
crypto/src/lib.rs 37.50% <ø> (ø)
crypto/src/mls/client/mod.rs 83.25% <ø> (ø)
crypto/src/mls/mod.rs 66.50% <ø> (ø)
crypto/src/group_store.rs 70.14% <75.00%> (ø)

Continue to review full report in Codecov by Sentry.

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

@github-actions GitHub Actions
Copy link

🐰 Bencher Report

Branchprgn/build/tokio-locks-experiment
Testbedubuntu-latest
Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
Commit add f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		18,242,000.00
Commit add f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		6,695,500.00
Commit add f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		9,103,400.00
Commit add f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		11,303,000.00
Commit add f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		14,498,000.00
Commit add f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		16,205,000.00
Commit add f(number clients)/cs1/mem/1002📈 view plot
🚷 view threshold		986,690,000.00
Commit add f(number clients)/cs1/mem/2📈 view plot
🚷 view threshold		6,729,100.00
Commit add f(number clients)/cs1/mem/202📈 view plot
🚷 view threshold		84,344,000.00
Commit add f(number clients)/cs1/mem/402📈 view plot
🚷 view threshold		219,570,000.00
Commit add f(number clients)/cs1/mem/602📈 view plot
🚷 view threshold		424,270,000.00
Commit add f(number clients)/cs1/mem/802📈 view plot
🚷 view threshold		673,350,000.00
Commit pending proposals f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		115,370,000.00
Commit pending proposals f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		28,092,000.00
Commit pending proposals f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		45,657,000.00
Commit pending proposals f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		60,358,000.00
Commit pending proposals f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		78,983,000.00
Commit pending proposals f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		94,042,000.00
Commit pending proposals f(pending size)/cs1/mem/1📈 view plot
🚷 view threshold		18,076,000.00
Commit pending proposals f(pending size)/cs1/mem/101📈 view plot
🚷 view threshold		114,100,000.00
Commit pending proposals f(pending size)/cs1/mem/21📈 view plot
🚷 view threshold		34,828,000.00
Commit pending proposals f(pending size)/cs1/mem/41📈 view plot
🚷 view threshold		55,955,000.00
Commit pending proposals f(pending size)/cs1/mem/61📈 view plot
🚷 view threshold		74,873,000.00
Commit pending proposals f(pending size)/cs1/mem/81📈 view plot
🚷 view threshold		94,392,000.00
Commit remove f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		26,609,000.00
Commit remove f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		6,655,700.00
Commit remove f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		8,606,900.00
Commit remove f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		11,402,000.00
Commit remove f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		16,816,000.00
Commit remove f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		21,690,000.00
Commit remove f(number clients)/cs1/mem/1002📈 view plot
🚷 view threshold		29,687,000.00
Commit remove f(number clients)/cs1/mem/2📈 view plot
🚷 view threshold		135,960,000.00
Commit remove f(number clients)/cs1/mem/202📈 view plot
🚷 view threshold		113,720,000.00
Commit remove f(number clients)/cs1/mem/402📈 view plot
🚷 view threshold		92,312,000.00
Commit remove f(number clients)/cs1/mem/602📈 view plot
🚷 view threshold		70,876,000.00
Commit remove f(number clients)/cs1/mem/802📈 view plot
🚷 view threshold		50,256,000.00
Commit update f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		135,320,000.00
Commit update f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		6,710,100.00
Commit update f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		32,998,000.00
Commit update f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		59,406,000.00
Commit update f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		85,364,000.00
Commit update f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		110,470,000.00
Count KeyPackage/cs1/mem/1002📈 view plot
🚷 view threshold		9,921,400.00
Count KeyPackage/cs1/mem/2📈 view plot
🚷 view threshold		6,293,000.00
Count KeyPackage/cs1/mem/202📈 view plot
🚷 view threshold		6,769,800.00
Count KeyPackage/cs1/mem/402📈 view plot
🚷 view threshold		7,569,400.00
Count KeyPackage/cs1/mem/602📈 view plot
🚷 view threshold		8,304,700.00
Count KeyPackage/cs1/mem/802📈 view plot
🚷 view threshold		9,040,100.00
Create group/cs1/mem📈 view plot
🚷 view threshold		6,408,300.00
Decrypt f(msg size)/cs1/mem/10📈 view plot
🚷 view threshold		6,174,200.00
Decrypt f(msg size)/cs1/mem/10010📈 view plot
🚷 view threshold		6,185,500.00
Decrypt f(msg size)/cs1/mem/2010📈 view plot
🚷 view threshold		6,114,900.00
Decrypt f(msg size)/cs1/mem/4010📈 view plot
🚷 view threshold		6,100,100.00
Decrypt f(msg size)/cs1/mem/6010📈 view plot
🚷 view threshold		6,156,000.00
Decrypt f(msg size)/cs1/mem/8010📈 view plot
🚷 view threshold		6,217,200.00
Encrypt f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		8,784,600.00
Encrypt f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		6,263,500.00
Encrypt f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		6,955,800.00
Encrypt f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		7,330,700.00
Encrypt f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		8,155,600.00
Encrypt f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		8,291,600.00
Encrypt f(msg size)/cs1/mem/10📈 view plot
🚷 view threshold		8,824,900.00
Encrypt f(msg size)/cs1/mem/10010📈 view plot
🚷 view threshold		8,763,100.00
Encrypt f(msg size)/cs1/mem/2010📈 view plot
🚷 view threshold		8,700,800.00
Encrypt f(msg size)/cs1/mem/4010📈 view plot
🚷 view threshold		8,681,700.00
Encrypt f(msg size)/cs1/mem/6010📈 view plot
🚷 view threshold		8,709,400.00
Encrypt f(msg size)/cs1/mem/8010📈 view plot
🚷 view threshold		8,844,300.00
Generate KeyPackage f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		243,550,000.00
Generate KeyPackage f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		6,651,400.00
Generate KeyPackage f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		33,272,000.00
Generate KeyPackage f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		86,026,000.00
Generate KeyPackage f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		139,800,000.00
Generate KeyPackage f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		192,100,000.00
Join from external commit f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		244,330,000.00
Join from external commit f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		7,632,200.00
Join from external commit f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		55,738,000.00
Join from external commit f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		102,490,000.00
Join from external commit f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		154,060,000.00
Join from external commit f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		197,910,000.00
Join from welcome f(group size)/cs1/mem/1002📈 view plot
🚷 view threshold		111,780,000.00
Join from welcome f(group size)/cs1/mem/2📈 view plot
🚷 view threshold		6,983,600.00
Join from welcome f(group size)/cs1/mem/202📈 view plot
🚷 view threshold		28,068,000.00
Join from welcome f(group size)/cs1/mem/402📈 view plot
🚷 view threshold		49,128,000.00
Join from welcome f(group size)/cs1/mem/602📈 view plot
🚷 view threshold		69,794,000.00
Join from welcome f(group size)/cs1/mem/802📈 view plot
🚷 view threshold		91,139,000.00
Mls vs Proteus: add/MLS/mem/1📈 view plot
🚷 view threshold		6,690,300.00
Mls vs Proteus: add/MLS/mem/101📈 view plot
🚷 view threshold		8,032,400.00
Mls vs Proteus: add/MLS/mem/21📈 view plot
🚷 view threshold		7,122,300.00
Mls vs Proteus: add/MLS/mem/41📈 view plot
🚷 view threshold		7,265,200.00
Mls vs Proteus: add/MLS/mem/61📈 view plot
🚷 view threshold		7,622,300.00
Mls vs Proteus: add/MLS/mem/81📈 view plot
🚷 view threshold		7,949,800.00
Mls vs Proteus: add/Proteus/mem/1📈 view plot
🚷 view threshold		6,409,500.00
Mls vs Proteus: add/Proteus/mem/101📈 view plot
🚷 view threshold		44,292,000.00
Mls vs Proteus: add/Proteus/mem/21📈 view plot
🚷 view threshold		14,160,000.00
Mls vs Proteus: add/Proteus/mem/41📈 view plot
🚷 view threshold		21,727,000.00
Mls vs Proteus: add/Proteus/mem/61📈 view plot
🚷 view threshold		29,341,000.00
Mls vs Proteus: add/Proteus/mem/81📈 view plot
🚷 view threshold		36,965,000.00
Mls vs Proteus: encrypt/MLS/mem/1📈 view plot
🚷 view threshold		6,251,700.00
Mls vs Proteus: encrypt/MLS/mem/101📈 view plot
🚷 view threshold		6,718,900.00
Mls vs Proteus: encrypt/MLS/mem/21📈 view plot
🚷 view threshold		6,439,500.00
Mls vs Proteus: encrypt/MLS/mem/41📈 view plot
🚷 view threshold		6,510,800.00
Mls vs Proteus: encrypt/MLS/mem/61📈 view plot
🚷 view threshold		6,508,000.00
Mls vs Proteus: encrypt/MLS/mem/81📈 view plot
🚷 view threshold		6,479,700.00
Mls vs Proteus: encrypt/Proteus/mem/1📈 view plot
🚷 view threshold		6,118,600.00
Mls vs Proteus: encrypt/Proteus/mem/101📈 view plot
🚷 view threshold		15,820,000.00
Mls vs Proteus: encrypt/Proteus/mem/21📈 view plot
🚷 view threshold		8,144,700.00
Mls vs Proteus: encrypt/Proteus/mem/41📈 view plot
🚷 view threshold		10,300,000.00
Mls vs Proteus: encrypt/Proteus/mem/61📈 view plot
🚷 view threshold		12,090,000.00
Mls vs Proteus: encrypt/Proteus/mem/81📈 view plot
🚷 view threshold		14,082,000.00
Mls vs Proteus: remove/MLS/mem/1📈 view plot
🚷 view threshold		20,287,000.00
Mls vs Proteus: remove/MLS/mem/101📈 view plot
🚷 view threshold		8,273,100.00
Mls vs Proteus: remove/MLS/mem/21📈 view plot
🚷 view threshold		17,724,000.00
Mls vs Proteus: remove/MLS/mem/41📈 view plot
🚷 view threshold		15,148,000.00
Mls vs Proteus: remove/MLS/mem/61📈 view plot
🚷 view threshold		12,950,000.00
Mls vs Proteus: remove/MLS/mem/81📈 view plot
🚷 view threshold		10,532,000.00
Mls vs Proteus: remove/Proteus/mem/1📈 view plot
🚷 view threshold		6,001,300.00
Mls vs Proteus: remove/Proteus/mem/101📈 view plot
🚷 view threshold		7,438,400.00
Mls vs Proteus: remove/Proteus/mem/21📈 view plot
🚷 view threshold		6,313,200.00
Mls vs Proteus: remove/Proteus/mem/41📈 view plot
🚷 view threshold		6,589,300.00
Mls vs Proteus: remove/Proteus/mem/61📈 view plot
🚷 view threshold		6,882,900.00
Mls vs Proteus: remove/Proteus/mem/81📈 view plot
🚷 view threshold		7,099,800.00
Mls vs Proteus: update/MLS/mem/1📈 view plot
🚷 view threshold		6,795,200.00
Mls vs Proteus: update/MLS/mem/101📈 view plot
🚷 view threshold		20,261,000.00
Mls vs Proteus: update/MLS/mem/21📈 view plot
🚷 view threshold		9,612,200.00
Mls vs Proteus: update/MLS/mem/41📈 view plot
🚷 view threshold		12,417,000.00
Mls vs Proteus: update/MLS/mem/61📈 view plot
🚷 view threshold		14,680,000.00
Mls vs Proteus: update/MLS/mem/81📈 view plot
🚷 view threshold		17,451,000.00
Mls vs Proteus: update/Proteus/mem/1📈 view plot
🚷 view threshold		6,411,200.00
Mls vs Proteus: update/Proteus/mem/101📈 view plot
🚷 view threshold		45,922,000.00
Mls vs Proteus: update/Proteus/mem/21📈 view plot
🚷 view threshold		14,393,000.00
Mls vs Proteus: update/Proteus/mem/41📈 view plot
🚷 view threshold		22,369,000.00
Mls vs Proteus: update/Proteus/mem/61📈 view plot
🚷 view threshold		30,383,000.00
Mls vs Proteus: update/Proteus/mem/81📈 view plot
🚷 view threshold		38,332,000.00
🐰 View full continuous benchmarking report in Bencher

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@coriolinus @codecov-commenter