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

add package to support metadata #1193

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

Conversation

Shourya742
Copy link
Contributor

closes #1192

Copy link
Contributor

github-actions bot commented Oct 8, 2024

🐰 Bencher Report

Branch1193/merge
Testbedsv2

⚠️ 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-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
client_sv2_handle_message_common📈 view plot
⚠️ NO THRESHOLD
44.89
client_sv2_handle_message_mining📈 view plot
⚠️ NO THRESHOLD
77.50
client_sv2_mining_message_submit_standard📈 view plot
⚠️ NO THRESHOLD
14.75
client_sv2_mining_message_submit_standard_serialize📈 view plot
⚠️ NO THRESHOLD
263.86
client_sv2_mining_message_submit_standard_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
599.53
client_sv2_open_channel📈 view plot
⚠️ NO THRESHOLD
166.89
client_sv2_open_channel_serialize📈 view plot
⚠️ NO THRESHOLD
271.60
client_sv2_open_channel_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
381.44
client_sv2_setup_connection📈 view plot
⚠️ NO THRESHOLD
162.91
client_sv2_setup_connection_serialize📈 view plot
⚠️ NO THRESHOLD
444.30
client_sv2_setup_connection_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
1,005.00
🐰 View full continuous benchmarking report in Bencher

Copy link
Contributor

github-actions bot commented Oct 8, 2024

🐰 Bencher Report

Branch1193/merge
Testbedsv1

⚠️ 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-thresholds CLI flag.

Click to view all benchmark results
BenchmarkLatencynanoseconds (ns)
client-submit-serialize📈 view plot
⚠️ NO THRESHOLD
6,774.30
client-submit-serialize-deserialize📈 view plot
⚠️ NO THRESHOLD
7,793.40
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle📈 view plot
⚠️ NO THRESHOLD
8,351.40
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle📈 view plot
⚠️ NO THRESHOLD
914.46
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize📈 view plot
⚠️ NO THRESHOLD
720.77
client-sv1-authorize-serialize/client-sv1-authorize-serialize📈 view plot
⚠️ NO THRESHOLD
247.60
client-sv1-get-authorize/client-sv1-get-authorize📈 view plot
⚠️ NO THRESHOLD
158.12
client-sv1-get-submit📈 view plot
⚠️ NO THRESHOLD
6,585.90
client-sv1-get-subscribe/client-sv1-get-subscribe📈 view plot
⚠️ NO THRESHOLD
275.24
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle📈 view plot
⚠️ NO THRESHOLD
770.28
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize📈 view plot
⚠️ NO THRESHOLD
630.95
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize📈 view plot
⚠️ NO THRESHOLD
205.54
🐰 View full continuous benchmarking report in Bencher

Copy link
Contributor

github-actions bot commented Oct 8, 2024

🐰 Bencher Report

Branch1193/merge
Testbedsv1

⚠️ 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-thresholds CLI flag.

Click to view all benchmark results
BenchmarkEstimated Cyclesestimated cyclesInstructionsinstructionsL1 AccessesaccessesL2 AccessesaccessesRAM Accessesaccesses
get_authorize📈 view plot
⚠️ NO THRESHOLD
8,587.00📈 view plot
⚠️ NO THRESHOLD
3,772.00📈 view plot
⚠️ NO THRESHOLD
5,292.00📈 view plot
⚠️ NO THRESHOLD
8.00📈 view plot
⚠️ NO THRESHOLD
93.00
get_submit📈 view plot
⚠️ NO THRESHOLD
95,464.00📈 view plot
⚠️ NO THRESHOLD
59,522.00📈 view plot
⚠️ NO THRESHOLD
85,509.00📈 view plot
⚠️ NO THRESHOLD
45.00📈 view plot
⚠️ NO THRESHOLD
278.00
get_subscribe📈 view plot
⚠️ NO THRESHOLD
8,102.00📈 view plot
⚠️ NO THRESHOLD
2,848.00📈 view plot
⚠️ NO THRESHOLD
3,982.00📈 view plot
⚠️ NO THRESHOLD
12.00📈 view plot
⚠️ NO THRESHOLD
116.00
serialize_authorize📈 view plot
⚠️ NO THRESHOLD
12,366.00📈 view plot
⚠️ NO THRESHOLD
5,343.00📈 view plot
⚠️ NO THRESHOLD
7,456.00📈 view plot
⚠️ NO THRESHOLD
9.00📈 view plot
⚠️ NO THRESHOLD
139.00
serialize_deserialize_authorize📈 view plot
⚠️ NO THRESHOLD
24,889.00📈 view plot
⚠️ NO THRESHOLD
9,920.00📈 view plot
⚠️ NO THRESHOLD
14,019.00📈 view plot
⚠️ NO THRESHOLD
32.00📈 view plot
⚠️ NO THRESHOLD
306.00
serialize_deserialize_handle_authorize📈 view plot
⚠️ NO THRESHOLD
30,431.00📈 view plot
⚠️ NO THRESHOLD
12,097.00📈 view plot
⚠️ NO THRESHOLD
17,141.00📈 view plot
⚠️ NO THRESHOLD
54.00📈 view plot
⚠️ NO THRESHOLD
372.00
serialize_deserialize_handle_submit📈 view plot
⚠️ NO THRESHOLD
126,467.00📈 view plot
⚠️ NO THRESHOLD
73,363.00📈 view plot
⚠️ NO THRESHOLD
105,197.00📈 view plot
⚠️ NO THRESHOLD
110.00📈 view plot
⚠️ NO THRESHOLD
592.00
serialize_deserialize_handle_subscribe📈 view plot
⚠️ NO THRESHOLD
27,987.00📈 view plot
⚠️ NO THRESHOLD
9,666.00📈 view plot
⚠️ NO THRESHOLD
13,677.00📈 view plot
⚠️ NO THRESHOLD
62.00📈 view plot
⚠️ NO THRESHOLD
400.00
serialize_deserialize_submit📈 view plot
⚠️ NO THRESHOLD
115,403.00📈 view plot
⚠️ NO THRESHOLD
68,223.00📈 view plot
⚠️ NO THRESHOLD
97,938.00📈 view plot
⚠️ NO THRESHOLD
63.00📈 view plot
⚠️ NO THRESHOLD
490.00
serialize_deserialize_subscribe📈 view plot
⚠️ NO THRESHOLD
23,423.00📈 view plot
⚠️ NO THRESHOLD
8,225.00📈 view plot
⚠️ NO THRESHOLD
11,593.00📈 view plot
⚠️ NO THRESHOLD
35.00📈 view plot
⚠️ NO THRESHOLD
333.00
serialize_submit📈 view plot
⚠️ NO THRESHOLD
99,857.00📈 view plot
⚠️ NO THRESHOLD
61,566.00📈 view plot
⚠️ NO THRESHOLD
88,352.00📈 view plot
⚠️ NO THRESHOLD
47.00📈 view plot
⚠️ NO THRESHOLD
322.00
serialize_subscribe📈 view plot
⚠️ NO THRESHOLD
11,431.00📈 view plot
⚠️ NO THRESHOLD
4,195.00📈 view plot
⚠️ NO THRESHOLD
5,841.00📈 view plot
⚠️ NO THRESHOLD
12.00📈 view plot
⚠️ NO THRESHOLD
158.00
🐰 View full continuous benchmarking report in Bencher

Copy link
Contributor

github-actions bot commented Oct 8, 2024

🐰 Bencher Report

Branch1193/merge
Testbedsv2

⚠️ 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-thresholds CLI flag.

Click to view all benchmark results
BenchmarkEstimated Cyclesestimated cyclesInstructionsinstructionsL1 AccessesaccessesL2 AccessesaccessesRAM Accessesaccesses
client_sv2_handle_message_common📈 view plot
⚠️ NO THRESHOLD
2,171.00📈 view plot
⚠️ NO THRESHOLD
473.00📈 view plot
⚠️ NO THRESHOLD
731.00📈 view plot
⚠️ NO THRESHOLD
8.00📈 view plot
⚠️ NO THRESHOLD
40.00
client_sv2_handle_message_mining📈 view plot
⚠️ NO THRESHOLD
8,216.00📈 view plot
⚠️ NO THRESHOLD
2,137.00📈 view plot
⚠️ NO THRESHOLD
3,156.00📈 view plot
⚠️ NO THRESHOLD
39.00📈 view plot
⚠️ NO THRESHOLD
139.00
client_sv2_mining_message_submit_standard📈 view plot
⚠️ NO THRESHOLD
6,353.00📈 view plot
⚠️ NO THRESHOLD
1,750.00📈 view plot
⚠️ NO THRESHOLD
2,548.00📈 view plot
⚠️ NO THRESHOLD
19.00📈 view plot
⚠️ NO THRESHOLD
106.00
client_sv2_mining_message_submit_standard_serialize📈 view plot
⚠️ NO THRESHOLD
14,716.00📈 view plot
⚠️ NO THRESHOLD
4,694.00📈 view plot
⚠️ NO THRESHOLD
6,756.00📈 view plot
⚠️ NO THRESHOLD
45.00📈 view plot
⚠️ NO THRESHOLD
221.00
client_sv2_mining_message_submit_standard_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
27,536.00📈 view plot
⚠️ NO THRESHOLD
10,585.00📈 view plot
⚠️ NO THRESHOLD
15,401.00📈 view plot
⚠️ NO THRESHOLD
82.00📈 view plot
⚠️ NO THRESHOLD
335.00
client_sv2_open_channel📈 view plot
⚠️ NO THRESHOLD
4,401.00📈 view plot
⚠️ NO THRESHOLD
1,461.00📈 view plot
⚠️ NO THRESHOLD
2,161.00📈 view plot
⚠️ NO THRESHOLD
7.00📈 view plot
⚠️ NO THRESHOLD
63.00
client_sv2_open_channel_serialize📈 view plot
⚠️ NO THRESHOLD
13,970.00📈 view plot
⚠️ NO THRESHOLD
5,064.00📈 view plot
⚠️ NO THRESHOLD
7,330.00📈 view plot
⚠️ NO THRESHOLD
33.00📈 view plot
⚠️ NO THRESHOLD
185.00
client_sv2_open_channel_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
22,729.00📈 view plot
⚠️ NO THRESHOLD
8,027.00📈 view plot
⚠️ NO THRESHOLD
11,674.00📈 view plot
⚠️ NO THRESHOLD
76.00📈 view plot
⚠️ NO THRESHOLD
305.00
client_sv2_setup_connection📈 view plot
⚠️ NO THRESHOLD
4,741.00📈 view plot
⚠️ NO THRESHOLD
1,502.00📈 view plot
⚠️ NO THRESHOLD
2,276.00📈 view plot
⚠️ NO THRESHOLD
10.00📈 view plot
⚠️ NO THRESHOLD
69.00
client_sv2_setup_connection_serialize📈 view plot
⚠️ NO THRESHOLD
16,162.00📈 view plot
⚠️ NO THRESHOLD
5,963.00📈 view plot
⚠️ NO THRESHOLD
8,667.00📈 view plot
⚠️ NO THRESHOLD
36.00📈 view plot
⚠️ NO THRESHOLD
209.00
client_sv2_setup_connection_serialize_deserialize📈 view plot
⚠️ NO THRESHOLD
35,536.00📈 view plot
⚠️ NO THRESHOLD
14,855.00📈 view plot
⚠️ NO THRESHOLD
21,826.00📈 view plot
⚠️ NO THRESHOLD
89.00📈 view plot
⚠️ NO THRESHOLD
379.00
🐰 View full continuous benchmarking report in Bencher

@plebhash
Copy link
Collaborator

plebhash commented Oct 8, 2024

hmm I'm not sure we want to have empty files to just to suppress warnings, that feels like the opposite direction

see #1194

@Shourya742
Copy link
Contributor Author

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.

@plebhash
Copy link
Collaborator

plebhash commented Oct 8, 2024

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 Cargo.toml files to each crate Cargo.toml so that we don't lose the metadata

the main point is that those fields need to be removed from the workspace Cargo.toml files so that they don't trigger the warnings

and we should not suppress the warnings by creating empty packages, that definitely feels like the wrong direction

@plebhash plebhash mentioned this pull request Oct 8, 2024
@Shourya742 Shourya742 force-pushed the 2024-10-remove-manifest-warning branch from 883eadb to 0cbc1dd Compare October 9, 2024 04:44
Copy link
Contributor

@jbesraa jbesraa left a comment

Choose a reason for hiding this comment

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

Few nits:

@@ -1,7 +1,7 @@
[package]
name = "interop-cpp"
version = "0.1.0"
authors = ["fi3 <email@email.org>"]
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need @Fi3 approval for those author changes..

Copy link
Contributor

Choose a reason for hiding this comment

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

..why do we actually need to change this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will wait for @Fi3 confirmation on this.

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"
Copy link
Contributor

Choose a reason for hiding this comment

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

this is or, not and

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't get it, Can you elaborate?

@Shourya742
Copy link
Contributor Author

Few nits:

  • It makes sense to link to the crate documentation, but for crates that are not yet published, we can continue pointing to this.
  • As for the descriptions, I can add a one-liner for each crate. Do you think adding a description here is necessary, considering most people refer to the documentation or README? Alternatively, we could remove it altogether.

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.

cargo warnings for unused manifest keys
3 participants