move metadata away from workspace manifests #1193
Conversation
Bencher Report
|
| Branch | 1193/merge |
| Testbed | sv1 |
⚠️ WARNING: The following Measure does not have a Threshold. Without a Threshold, no Alerts will ever be generated!Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsCLI flag.
Click to view all benchmark results
| Benchmark | Latency | nanoseconds (ns) |
|---|---|---|
| client-submit-serialize | 📈 view plot | 6,676.50 |
| client-submit-serialize-deserialize | 📈 view plot | 7,778.30 |
| client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle | 📈 view plot | 8,382.70 |
| client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle | 📈 view plot | 925.06 |
| client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize | 📈 view plot | 766.33 |
| client-sv1-authorize-serialize/client-sv1-authorize-serialize | 📈 view plot | 245.62 |
| client-sv1-get-authorize/client-sv1-get-authorize | 📈 view plot | 157.05 |
| client-sv1-get-submit | 📈 view plot | 6,493.60 |
| client-sv1-get-subscribe/client-sv1-get-subscribe | 📈 view plot | 275.39 |
| client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle | 📈 view plot | 762.24 |
| client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize | 📈 view plot | 631.14 |
| client-sv1-subscribe-serialize/client-sv1-subscribe-serialize | 📈 view plot | 203.33 |
|
| Branch | 1193/merge |
| Testbed | sv1 |
⚠️ WARNING: The following Measures do not have a Threshold. Without a Threshold, no Alerts will ever be generated!Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsCLI flag.
Click to view all benchmark results
| Benchmark | Estimated Cycles | estimated cycles | Instructions | instructions | L1 Accesses | accesses | L2 Accesses | accesses | RAM Accesses | accesses |
|---|---|---|---|---|---|---|---|---|---|---|
| get_authorize | 📈 view plot | 8,605.00 | 📈 view plot | 3,772.00 | 📈 view plot | 5,295.00 | 📈 view plot | 4.00 | 📈 view plot | 94.00 |
| get_submit | 📈 view plot | 95,490.00 | 📈 view plot | 59,522.00 | 📈 view plot | 85,510.00 | 📈 view plot | 43.00 | 📈 view plot | 279.00 |
| get_subscribe | 📈 view plot | 8,128.00 | 📈 view plot | 2,848.00 | 📈 view plot | 3,983.00 | 📈 view plot | 10.00 | 📈 view plot | 117.00 |
| serialize_authorize | 📈 view plot | 12,384.00 | 📈 view plot | 5,343.00 | 📈 view plot | 7,459.00 | 📈 view plot | 5.00 | 📈 view plot | 140.00 |
| serialize_deserialize_authorize | 📈 view plot | 24,911.00 | 📈 view plot | 9,920.00 | 📈 view plot | 14,021.00 | 📈 view plot | 29.00 | 📈 view plot | 307.00 |
| serialize_deserialize_handle_authorize | 📈 view plot | 30,461.00 | 📈 view plot | 12,097.00 | 📈 view plot | 17,141.00 | 📈 view plot | 53.00 | 📈 view plot | 373.00 |
| serialize_deserialize_handle_submit | 📈 view plot | 126,467.00 | 📈 view plot | 73,363.00 | 📈 view plot | 105,197.00 | 📈 view plot | 110.00 | 📈 view plot | 592.00 |
| serialize_deserialize_handle_subscribe | 📈 view plot | 28,013.00 | 📈 view plot | 9,666.00 | 📈 view plot | 13,678.00 | 📈 view plot | 60.00 | 📈 view plot | 401.00 |
| serialize_deserialize_submit | 📈 view plot | 115,403.00 | 📈 view plot | 68,223.00 | 📈 view plot | 97,938.00 | 📈 view plot | 63.00 | 📈 view plot | 490.00 |
| serialize_deserialize_subscribe | 📈 view plot | 23,449.00 | 📈 view plot | 8,225.00 | 📈 view plot | 11,594.00 | 📈 view plot | 33.00 | 📈 view plot | 334.00 |
| serialize_submit | 📈 view plot | 99,883.00 | 📈 view plot | 61,566.00 | 📈 view plot | 88,353.00 | 📈 view plot | 45.00 | 📈 view plot | 323.00 |
| serialize_subscribe | 📈 view plot | 11,457.00 | 📈 view plot | 4,195.00 | 📈 view plot | 5,842.00 | 📈 view plot | 10.00 | 📈 view plot | 159.00 |
|
| Branch | 1193/merge |
| Testbed | sv2 |
⚠️ WARNING: The following Measures do not have a Threshold. Without a Threshold, no Alerts will ever be generated!Click here to create a new Threshold
For more information, see the Threshold documentation.
To only post results if a Threshold exists, set the--ci-only-thresholdsCLI flag.
Click to view all benchmark results
| Benchmark | Estimated Cycles | estimated cycles | Instructions | instructions | L1 Accesses | accesses | L2 Accesses | accesses | RAM Accesses | accesses |
|---|---|---|---|---|---|---|---|---|---|---|
| client_sv2_handle_message_common | 📈 view plot | 2,171.00 | 📈 view plot | 473.00 | 📈 view plot | 731.00 | 📈 view plot | 8.00 | 📈 view plot | 40.00 |
| client_sv2_handle_message_mining | 📈 view plot | 8,216.00 | 📈 view plot | 2,137.00 | 📈 view plot | 3,156.00 | 📈 view plot | 39.00 | 📈 view plot | 139.00 |
| client_sv2_mining_message_submit_standard | 📈 view plot | 6,353.00 | 📈 view plot | 1,750.00 | 📈 view plot | 2,548.00 | 📈 view plot | 19.00 | 📈 view plot | 106.00 |
| client_sv2_mining_message_submit_standard_serialize | 📈 view plot | 14,716.00 | 📈 view plot | 4,694.00 | 📈 view plot | 6,756.00 | 📈 view plot | 45.00 | 📈 view plot | 221.00 |
| client_sv2_mining_message_submit_standard_serialize_deserialize | 📈 view plot | 27,536.00 | 📈 view plot | 10,585.00 | 📈 view plot | 15,401.00 | 📈 view plot | 82.00 | 📈 view plot | 335.00 |
| client_sv2_open_channel | 📈 view plot | 4,401.00 | 📈 view plot | 1,461.00 | 📈 view plot | 2,161.00 | 📈 view plot | 7.00 | 📈 view plot | 63.00 |
| client_sv2_open_channel_serialize | 📈 view plot | 13,970.00 | 📈 view plot | 5,064.00 | 📈 view plot | 7,330.00 | 📈 view plot | 33.00 | 📈 view plot | 185.00 |
| client_sv2_open_channel_serialize_deserialize | 📈 view plot | 22,729.00 | 📈 view plot | 8,027.00 | 📈 view plot | 11,674.00 | 📈 view plot | 76.00 | 📈 view plot | 305.00 |
| client_sv2_setup_connection | 📈 view plot | 4,741.00 | 📈 view plot | 1,502.00 | 📈 view plot | 2,276.00 | 📈 view plot | 10.00 | 📈 view plot | 69.00 |
| client_sv2_setup_connection_serialize | 📈 view plot | 16,162.00 | 📈 view plot | 5,963.00 | 📈 view plot | 8,667.00 | 📈 view plot | 36.00 | 📈 view plot | 209.00 |
| client_sv2_setup_connection_serialize_deserialize | 📈 view plot | 35,536.00 | 📈 view plot | 14,855.00 | 📈 view plot | 21,826.00 | 📈 view plot | 89.00 | 📈 view plot | 379.00 |
|
hmm I'm not sure we want to have empty files to just to suppress warnings, that feels like the opposite direction see #1194 |
@plebhash, I’m unsure about removing the metadata from the root Cargo file. Another possible way to retain the metadata is by adding it to each crate within the workspace, but that seems excessive. I’m fine with either approach. |
|
I didn't notice you were assigned to #1192 feel free to modify this PR instead of #1194 but yeah, we can "transfer" the relevant fields from the workspace the main point is that those fields need to be removed from the workspace and we should not suppress the warnings by creating empty packages, that definitely feels like the wrong direction |
Shourya742
force-pushed
the
2024-10-remove-manifest-warning
branch
from
883eadb to
0cbc1dd
Compare
There was a problem hiding this comment.
Few nits:
- What do you think about adding
descriptionas well? documentationis always linking to "https://github.com/stratum-mining/stratum", why not link to the specific crate docs on docs.rs?
| description = "API for bridging SV1 miners to SV2 pools" | ||
| license = "MIT OR Apache-2.0" | ||
| documentation = "https://github.com/stratum-mining/stratum" | ||
| license = "MIT + Apache-2.0" |
There was a problem hiding this comment.
I didn't get it, Can you elaborate?
There was a problem hiding this comment.
You changed the license wording from MIT <OR> Apache to MIT <AND> Apache, it should stay as OR (i.e., don't change OR to +)
There was a problem hiding this comment.
As it used + everywhere except this
There was a problem hiding this comment.
umm not really.. take a look into the files please. it is currently either MIT or MIT OR Apache and all of the changes modified it to MIT + Apache. Anyway, saying MIT AND Apache is confusing as AND implies both licenses forced together, while its either this or that, however the user wants.
There was a problem hiding this comment.
Actually, MIT + Apache means users can choose either the MIT or Apache 2.0 license. It doesn’t mean both licenses apply together, just that the user has the option to pick one.
There was a problem hiding this comment.
I do think it is confusing to use and but mean or.. but anyway this is not a big deal
There was a problem hiding this comment.
turns out this is actually a big deal 😛
|
Shourya742
force-pushed
the
2024-10-remove-manifest-warning
branch
3 times, most recently
from
e59c789 to
34cd893
Compare
I think all of our crates have a page on docs.rs. I would link to that, regardless if the docs are up to date or not. If no docs.rs page for a crate then yea, keeping as is seems reasonable. For the desc, a one liner would be great. Will make the cargo.toml more complete. |
Yup, followed the same. You can check, all the documentation are now pointing to their corresponding crate doc |
Shourya742
force-pushed
the
2024-10-remove-manifest-warning
branch
from
34cd893 to
5a7ecea
Compare
plebhash
changed the title
|
@Shourya742 looks like we got a conflict after merging #1198 |
Let me resolve locally... and push.. |
Shourya742
force-pushed
the
2024-10-remove-manifest-warning
branch
from
5a7ecea to
ce2f085
Compare
closes #1192