Merge protocols and roles workspaces
#1126
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbesraa
force-pushed
the
2024-08-21-workspaces
branch
2 times, most recently
from
a320023 to
1766776
Compare
This commits removes the `protocols` and `roles` workspaces and creates a new shared workspace for both of them. In practice we dont have a need to maintain two different workspaces and no benefit is derived from that. With moving to a shared workspaces, the naming of the parent folders was changed as will with `roles` becoming `stratum-roles` and `protocols` becoming `stratum-protocols` and same for `common` crate. This required a fix a across the codebase of the imports inside the different `Cargo.toml`s.
jbesraa
force-pushed
the
2024-08-21-workspaces
branch
from
1766776 to
8465b1f
Compare
|
| Report | Wed, August 21, 2024 at 09:26:28 UTC |
| Project | Stratum v2 (SRI) |
| Branch | 2024-08-21-workspaces |
| Testbed | sv2 |
🚨 6 ALERTS: Threshold Boundary Limits exceeded!
| Benchmark | Measure (units) | View | Value | Lower Boundary | Upper Boundary |
|---|---|---|---|---|---|
| client_sv2_mining_message_submit_standard_serialize_deserialize | Instructions (instructions) | 🚨 (view plot | view alert) | 10,676.00 (+1.23%) | 10,592.25 (100.79%) | |
| client_sv2_mining_message_submit_standard_serialize_deserialize | L1 Accesses (accesses) | 🚨 (view plot | view alert) | 15,540.00 (+1.27%) | 15,411.37 (100.83%) | |
| client_sv2_open_channel_serialize_deserialize | Instructions (instructions) | 🚨 (view plot | view alert) | 8,027.00 (+0.51%) | 8,015.26 (100.15%) | |
| client_sv2_open_channel_serialize_deserialize | L1 Accesses (accesses) | 🚨 (view plot | view alert) | 11,679.00 (+0.53%) | 11,655.20 (100.20%) | |
| client_sv2_setup_connection_serialize_deserialize | Instructions (instructions) | 🚨 (view plot | view alert) | 14,855.00 (+0.28%) | 14,842.76 (100.08%) | |
| client_sv2_setup_connection_serialize_deserialize | L1 Accesses (accesses) | 🚨 (view plot | view alert) | 21,830.00 (+0.35%) | 21,796.48 (100.15%) |
Click to view all benchmark results
| Benchmark | Estimated Cycles | Estimated Cycles Results estimated cycles | (Δ%) | Estimated Cycles Upper Boundary estimated cycles | (%) | Instructions | Instructions Results instructions | (Δ%) | Instructions Upper Boundary instructions | (%) | L1 Accesses | L1 Accesses Results accesses | (Δ%) | L1 Accesses Upper Boundary accesses | (%) | L2 Accesses | L2 Accesses Results accesses | (Δ%) | L2 Accesses Upper Boundary accesses | (%) | RAM Accesses | RAM Accesses Results accesses | (Δ%) | RAM Accesses Upper Boundary accesses | (%) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| client_sv2_handle_message_common | ✅ (view plot) | 2,091.00 (+1.77%) | 2,139.75 (97.72%) | ✅ (view plot) | 473.00 (+0.45%) | 486.03 (97.32%) | ✅ (view plot) | 736.00 (+0.52%) | 754.55 (97.54%) | ✅ (view plot) | 5.00 (-29.10%) | 11.32 (44.18%) | ✅ (view plot) | 38.00 (+3.33%) | 38.88 (97.74%) |
| client_sv2_handle_message_mining | ✅ (view plot) | 8,162.00 (-0.44%) | 8,334.58 (97.93%) | ✅ (view plot) | 2,137.00 (+0.41%) | 2,171.42 (98.41%) | ✅ (view plot) | 3,162.00 (+0.50%) | 3,215.99 (98.32%) | ✅ (view plot) | 34.00 (-11.17%) | 43.47 (78.22%) | ✅ (view plot) | 138.00 (-0.63%) | 141.93 (97.23%) |
| client_sv2_mining_message_submit_standard | ✅ (view plot) | 6,247.00 (-0.49%) | 6,392.64 (97.72%) | ✅ (view plot) | 1,750.00 (+0.01%) | 1,763.37 (99.24%) | ✅ (view plot) | 2,552.00 (-0.08%) | 2,575.28 (99.10%) | ✅ (view plot) | 18.00 (+2.15%) | 22.85 (78.76%) | ✅ (view plot) | 103.00 (-0.85%) | 106.93 (96.33%) |
| client_sv2_mining_message_submit_standard_serialize | ✅ (view plot) | 14,674.00 (-0.72%) | 15,032.58 (97.61%) | ✅ (view plot) | 4,694.00 (+0.00%) | 4,707.37 (99.72%) | ✅ (view plot) | 6,759.00 (+0.06%) | 6,774.98 (99.76%) | ✅ (view plot) | 43.00 (-8.78%) | 51.68 (83.20%) | ✅ (view plot) | 220.00 (-1.16%) | 229.88 (95.70%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | ✅ (view plot) | 27,665.00 (+0.63%) | 27,854.74 (99.32%) | 🚨 (view plot | view alert) | 10,676.00 (+1.23%) | 10,592.25 (100.79%) | 🚨 (view plot | view alert) | 15,540.00 (+1.27%) | 15,411.37 (100.83%) | ✅ (view plot) | 80.00 (-4.35%) | 89.07 (89.82%) | ✅ (view plot) | 335.00 (-0.02%) | 345.20 (97.04%) |
| client_sv2_open_channel | ✅ (view plot) | 4,333.00 (-3.41%) | 4,621.95 (93.75%) | ✅ (view plot) | 1,461.00 (+0.05%) | 1,473.99 (99.12%) | ✅ (view plot) | 2,163.00 (+0.47%) | 2,172.69 (99.55%) | ✅ (view plot) | 7.00 (-41.67%) | 15.51 (45.12%) | ✅ (view plot) | 61.00 (-6.08%) | 68.58 (88.95%) |
| client_sv2_open_channel_serialize | ✅ (view plot) | 13,944.00 (-1.89%) | 14,460.17 (96.43%) | ✅ (view plot) | 5,064.00 (+0.01%) | 5,076.99 (99.74%) | ✅ (view plot) | 7,329.00 (+0.15%) | 7,339.30 (99.86%) | ✅ (view plot) | 35.00 (-5.54%) | 41.30 (84.75%) | ✅ (view plot) | 184.00 (-4.00%) | 198.82 (92.55%) |
| client_sv2_open_channel_serialize_deserialize | ✅ (view plot) | 22,619.00 (-0.13%) | 23,018.96 (98.26%) | 🚨 (view plot | view alert) | 8,027.00 (+0.51%) | 8,015.26 (100.15%) | 🚨 (view plot | view alert) | 11,679.00 (+0.53%) | 11,655.20 (100.20%) | ✅ (view plot) | 74.00 (+0.37%) | 83.04 (89.11%) | ✅ (view plot) | 302.00 (-0.86%) | 314.64 (95.98%) |
| client_sv2_setup_connection | ✅ (view plot) | 4,661.00 (-0.81%) | 4,765.72 (97.80%) | ✅ (view plot) | 1,502.00 (+0.05%) | 1,514.99 (99.14%) | ✅ (view plot) | 2,281.00 (+0.18%) | 2,298.99 (99.22%) | ✅ (view plot) | 7.00 (-23.68%) | 13.94 (50.22%) | ✅ (view plot) | 67.00 (-1.32%) | 69.63 (96.23%) |
| client_sv2_setup_connection_serialize | ✅ (view plot) | 16,060.00 (-1.28%) | 16,484.30 (97.43%) | ✅ (view plot) | 5,963.00 (+0.01%) | 5,975.99 (99.78%) | ✅ (view plot) | 8,670.00 (+0.17%) | 8,677.09 (99.92%) | ✅ (view plot) | 36.00 (-19.51%) | 50.03 (71.96%) | ✅ (view plot) | 206.00 (-2.43%) | 217.21 (94.84%) |
| client_sv2_setup_connection_serialize_deserialize | ✅ (view plot) | 35,400.00 (-0.39%) | 35,750.06 (99.02%) | 🚨 (view plot | view alert) | 14,855.00 (+0.28%) | 14,842.76 (100.08%) | 🚨 (view plot | view alert) | 21,830.00 (+0.35%) | 21,796.48 (100.15%) | ✅ (view plot) | 89.00 (-10.58%) | 112.40 (79.18%) | ✅ (view plot) | 375.00 (-1.23%) | 384.40 (97.56%) |
Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help
|
| Report | Wed, August 21, 2024 at 09:26:29 UTC |
| Project | Stratum v2 (SRI) |
| Branch | 2024-08-21-workspaces |
| Testbed | sv2 |
🚨 1 ALERT: Threshold Boundary Limit exceeded!
| Benchmark | Measure (units) | View | Value | Lower Boundary | Upper Boundary |
|---|---|---|---|---|---|
| client_sv2_mining_message_submit_standard | Latency (nanoseconds (ns)) | 🚨 (view plot | view alert) | 14.73 (+0.53%) | 14.70 (100.20%) |
Click to view all benchmark results
| Benchmark | Latency | Latency Results nanoseconds (ns) | (Δ%) | Latency Upper Boundary nanoseconds (ns) | (%) |
|---|---|---|---|
| client_sv2_handle_message_common | ✅ (view plot) | 44.73 (+0.32%) | 45.33 (98.67%) |
| client_sv2_handle_message_mining | ✅ (view plot) | 73.40 (+0.47%) | 80.84 (90.80%) |
| client_sv2_mining_message_submit_standard | 🚨 (view plot | view alert) | 14.73 (+0.53%) | 14.70 (100.20%) |
| client_sv2_mining_message_submit_standard_serialize | ✅ (view plot) | 261.93 (-1.00%) | 283.86 (92.27%) |
| client_sv2_mining_message_submit_standard_serialize_deserialize | ✅ (view plot) | 589.30 (-0.71%) | 626.35 (94.08%) |
| client_sv2_open_channel | ✅ (view plot) | 162.27 (-1.91%) | 171.47 (94.64%) |
| client_sv2_open_channel_serialize | ✅ (view plot) | 273.56 (-3.26%) | 293.53 (93.20%) |
| client_sv2_open_channel_serialize_deserialize | ✅ (view plot) | 374.96 (-1.19%) | 427.20 (87.77%) |
| client_sv2_setup_connection | ✅ (view plot) | 158.38 (-3.36%) | 174.92 (90.55%) |
| client_sv2_setup_connection_serialize | ✅ (view plot) | 429.39 (-9.21%) | 505.87 (84.88%) |
| client_sv2_setup_connection_serialize_deserialize | ✅ (view plot) | 988.60 (+1.53%) | 1,045.26 (94.58%) |
Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help
|
| Report | Wed, August 21, 2024 at 09:26:29 UTC |
| Project | Stratum v2 (SRI) |
| Branch | 2024-08-21-workspaces |
| Testbed | sv1 |
🚨 7 ALERTS: Threshold Boundary Limits exceeded!
| Benchmark | Measure (units) | View | Value | Lower Boundary | Upper Boundary |
|---|---|---|---|---|---|
| serialize_deserialize_authorize | Estimated Cycles (estimated cycles) | 🚨 (view plot | view alert) | 24,787.00 (+1.28%) | 24,743.58 (100.18%) | |
| serialize_deserialize_authorize | RAM Accesses (accesses) | 🚨 (view plot | view alert) | 303.00 (+2.74%) | 298.62 (101.47%) | |
| serialize_deserialize_handle_authorize | RAM Accesses (accesses) | 🚨 (view plot | view alert) | 369.00 (+1.45%) | 367.52 (100.40%) | |
| serialize_deserialize_handle_subscribe | Estimated Cycles (estimated cycles) | 🚨 (view plot | view alert) | 27,929.00 (+1.72%) | 27,691.80 (100.86%) | |
| serialize_deserialize_handle_subscribe | RAM Accesses (accesses) | 🚨 (view plot | view alert) | 399.00 (+3.39%) | 390.76 (102.11%) | |
| serialize_deserialize_subscribe | Estimated Cycles (estimated cycles) | 🚨 (view plot | view alert) | 23,355.00 (+2.06%) | 23,176.88 (100.77%) | |
| serialize_deserialize_subscribe | RAM Accesses (accesses) | 🚨 (view plot | view alert) | 331.00 (+3.82%) | 323.62 (102.28%) |
Click to view all benchmark results
| Benchmark | Estimated Cycles | Estimated Cycles Results estimated cycles | (Δ%) | Estimated Cycles Upper Boundary estimated cycles | (%) | Instructions | Instructions Results instructions | (Δ%) | Instructions Upper Boundary instructions | (%) | L1 Accesses | L1 Accesses Results accesses | (Δ%) | L1 Accesses Upper Boundary accesses | (%) | L2 Accesses | L2 Accesses Results accesses | (Δ%) | L2 Accesses Upper Boundary accesses | (%) | RAM Accesses | RAM Accesses Results accesses | (Δ%) | RAM Accesses Upper Boundary accesses | (%) |
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| get_authorize | ✅ (view plot) | 8,537.00 (+1.26%) | 8,744.05 (97.63%) | ✅ (view plot) | 3,772.00 (+0.87%) | 3,864.38 (97.61%) | ✅ (view plot) | 5,297.00 (+0.99%) | 5,414.20 (97.84%) | ✅ (view plot) | 4.00 (-48.13%) | 10.66 (37.53%) | ✅ (view plot) | 92.00 (+2.32%) | 94.21 (97.65%) |
| get_submit | ✅ (view plot) | 95,396.00 (-0.16%) | 96,163.13 (99.20%) | ✅ (view plot) | 59,522.00 (+0.09%) | 59,779.04 (99.57%) | ✅ (view plot) | 85,511.00 (+0.12%) | 85,838.78 (99.62%) | ✅ (view plot) | 45.00 (-17.80%) | 62.87 (71.58%) | ✅ (view plot) | 276.00 (-2.05%) | 288.75 (95.58%) |
| get_subscribe | ✅ (view plot) | 8,060.00 (+1.08%) | 8,296.73 (97.15%) | ✅ (view plot) | 2,848.00 (+0.73%) | 2,948.57 (96.59%) | ✅ (view plot) | 3,985.00 (+0.86%) | 4,111.47 (96.92%) | ✅ (view plot) | 10.00 (-37.10%) | 20.42 (48.97%) | ✅ (view plot) | 115.00 (+2.08%) | 117.41 (97.95%) |
| serialize_authorize | ✅ (view plot) | 12,222.00 (+0.14%) | 12,532.23 (97.52%) | ✅ (view plot) | 5,343.00 (+0.61%) | 5,435.38 (98.30%) | ✅ (view plot) | 7,462.00 (+0.74%) | 7,577.33 (98.48%) | ✅ (view plot) | 7.00 (-33.60%) | 13.74 (50.95%) | ✅ (view plot) | 135.00 (-0.41%) | 140.71 (95.94%) |
| serialize_deserialize_authorize | 🚨 (view plot | view alert) | 24,787.00 (+1.28%) | 24,743.58 (100.18%) | ✅ (view plot) | 9,920.00 (+0.19%) | 10,035.06 (98.85%) | ✅ (view plot) | 14,022.00 (+0.37%) | 14,164.68 (98.99%) | ✅ (view plot) | 32.00 (-12.10%) | 41.85 (76.46%) | 🚨 (view plot | view alert) | 303.00 (+2.74%) | 298.62 (101.47%) |
| serialize_deserialize_handle_authorize | ✅ (view plot) | 30,309.00 (+0.56%) | 30,378.82 (99.77%) | ✅ (view plot) | 12,097.00 (+0.02%) | 12,218.55 (99.01%) | ✅ (view plot) | 17,149.00 (+0.19%) | 17,290.94 (99.18%) | ✅ (view plot) | 49.00 (-16.37%) | 64.99 (75.40%) | 🚨 (view plot | view alert) | 369.00 (+1.45%) | 367.52 (100.40%) |
| serialize_deserialize_handle_submit | ✅ (view plot) | 126,481.00 (+0.06%) | 127,073.69 (99.53%) | ✅ (view plot) | 73,363.00 (+0.15%) | 73,626.00 (99.64%) | ✅ (view plot) | 105,201.00 (+0.20%) | 105,523.64 (99.69%) | ✅ (view plot) | 105.00 (-12.12%) | 132.35 (79.34%) | ✅ (view plot) | 593.00 (-0.28%) | 600.56 (98.74%) |
| serialize_deserialize_handle_subscribe | 🚨 (view plot | view alert) | 27,929.00 (+1.72%) | 27,691.80 (100.86%) | ✅ (view plot) | 9,666.00 (+0.38%) | 9,751.12 (99.13%) | ✅ (view plot) | 13,684.00 (+0.47%) | 13,787.75 (99.25%) | ✅ (view plot) | 56.00 (-14.82%) | 73.81 (75.87%) | 🚨 (view plot | view alert) | 399.00 (+3.39%) | 390.76 (102.11%) |
| serialize_deserialize_submit | ✅ (view plot) | 115,369.00 (+0.24%) | 115,690.85 (99.72%) | ✅ (view plot) | 68,223.00 (+0.23%) | 68,411.79 (99.72%) | ✅ (view plot) | 97,939.00 (+0.28%) | 98,170.15 (99.76%) | ✅ (view plot) | 63.00 (-8.83%) | 76.10 (82.78%) | ✅ (view plot) | 489.00 (+0.20%) | 493.50 (99.09%) |
| serialize_deserialize_subscribe | 🚨 (view plot | view alert) | 23,355.00 (+2.06%) | 23,176.88 (100.77%) | ✅ (view plot) | 8,225.00 (+0.50%) | 8,306.97 (99.01%) | ✅ (view plot) | 11,595.00 (+0.58%) | 11,695.78 (99.14%) | ✅ (view plot) | 35.00 (-10.95%) | 44.05 (79.46%) | 🚨 (view plot | view alert) | 331.00 (+3.82%) | 323.62 (102.28%) |
| serialize_submit | ✅ (view plot) | 99,721.00 (-0.17%) | 100,498.32 (99.23%) | ✅ (view plot) | 61,566.00 (+0.08%) | 61,828.09 (99.58%) | ✅ (view plot) | 88,356.00 (+0.11%) | 88,691.99 (99.62%) | ✅ (view plot) | 47.00 (-15.07%) | 63.16 (74.42%) | ✅ (view plot) | 318.00 (-2.01%) | 330.87 (96.11%) |
| serialize_subscribe | ✅ (view plot) | 11,333.00 (+0.04%) | 11,643.29 (97.34%) | ✅ (view plot) | 4,195.00 (+0.49%) | 4,295.57 (97.66%) | ✅ (view plot) | 5,843.00 (+0.58%) | 5,970.41 (97.87%) | ✅ (view plot) | 13.00 (-19.69%) | 19.36 (67.14%) | ✅ (view plot) | 155.00 (-0.24%) | 160.53 (96.55%) |
Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help
|
| Report | Wed, August 21, 2024 at 09:26:30 UTC |
| Project | Stratum v2 (SRI) |
| Branch | 1126/merge |
| Testbed | sv1 |
Click to view all benchmark results
| Benchmark | Latency | Latency Results nanoseconds (ns) | (Δ%) | Latency Upper Boundary nanoseconds (ns) | (%) |
|---|---|---|---|
| client-submit-serialize | ✅ (view plot) | 6,821.00 (-1.75%) | 7,334.54 (93.00%) |
| client-submit-serialize-deserialize | ✅ (view plot) | 7,716.80 (-1.89%) | 8,335.43 (92.58%) |
| client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle | ✅ (view plot) | 8,347.30 (-1.00%) | 8,837.27 (94.46%) |
| client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle | ✅ (view plot) | 916.94 (+2.01%) | 927.41 (98.87%) |
| client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize | ✅ (view plot) | 714.04 (+2.38%) | 718.84 (99.33%) |
| client-sv1-authorize-serialize/client-sv1-authorize-serialize | ✅ (view plot) | 249.28 (+0.49%) | 255.16 (97.70%) |
| client-sv1-get-authorize/client-sv1-get-authorize | ✅ (view plot) | 158.20 (+0.66%) | 162.40 (97.41%) |
| client-sv1-get-submit | ✅ (view plot) | 6,694.50 (-0.24%) | 7,134.32 (93.84%) |
| client-sv1-get-subscribe/client-sv1-get-subscribe | ✅ (view plot) | 278.96 (+0.14%) | 291.04 (95.85%) |
| client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle | ✅ (view plot) | 744.41 (-0.34%) | 775.59 (95.98%) |
| client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize | ✅ (view plot) | 617.39 (+0.48%) | 637.45 (96.85%) |
| client-sv1-subscribe-serialize/client-sv1-subscribe-serialize | ✅ (view plot) | 204.43 (-1.10%) | 219.83 (92.99%) |
Bencher - Continuous Benchmarking
View Public Perf Page
Docs | Repo | Chat | Help
|
see my comment on related issue |
|
@Fi3 I dont honestly remember when we talked about splitting them into different git repos, but anyhow thats a much bigger task than just combining them into a single workspace. Things are way over-engineered at the moment without any justification. Whats wrong with combining them together? |
I think there is also an open issue about that by @plebhash ?
What do you mean by that? Btw we splitted the workspaces with the goal of putting protocols and roles in separate repos. The plan was somthing like that:
|
There is this proposal discussion for a repository split: #1069 (comment) But I'm not in a rush to execute this split just yet, because this has many implications and needs to be properly planned. For now, I'd be happy to keep the discussion going on a conceptual level, so we get consensus on the best repository structure and we can eventually plan accordingly for the execution in the future.
I think we are in the middle of 3 right now, via #845 And also #1120 will help bring more stability into the work that was done on 2, because we will be able to get more confidence that the roles implementations are correctly implementing the protocol spec. Once we move into 4 in the future, we will basically be trying to bring the work that was done in 2 into production readiness. Which ones we will focus first is a future discussion, and hopefully by that time we will also have more feedback from industry adopters, which will help us make those decisions. with regards to this actual PR, I'm not very opinionated in case I come across something that makes me realize this is needed I'll come back here and elaborate, but for now I'll focus on other tasks |
|
concept nack |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#1125
This commits removes the
protocolsandrolesworkspaces and creates a new shared workspace for both of them. In practice we dont have a need to maintain two different workspaces and no benefit is derived from that.With moving to a shared workspaces, the naming of the parent folders was changed as will with
rolesbecomingstratum-rolesandprotocolsbecomingstratum-protocolsand same forcommoncrate. This required a fix a across the codebase of the imports inside the differentCargo.tomls.