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

remove redundant Drop implementation from ITF TemplateProvider #1291

Merged
merged 1 commit into from
Dec 11, 2024
Merged

remove redundant Drop implementation from ITF TemplateProvider #1291

merged 1 commit into from
Dec 11, 2024

Conversation

plebhash
Copy link
Collaborator

@plebhash plebhash commented Dec 11, 2024 •
edited
Loading

replace #1290

close #1288

BitcoinD already implements the Drop trait

then we have a second Drop implementation for TemplateProvider:

stratum/roles/tests-integration/tests/common/mod.rs

Lines 167 to 171 in 5bcd54d

impl Drop for TemplateProvider {
fn drop(&mut self) {
self.stop();
}
}

this is not only redundant, but likely the root cause of our problems here, since it calls RPC stop 2x:

  • the first one kills the process
  • the second one fails, since the process is already dead

@plebhash plebhash requested review from Shourya742 and jbesraa and removed request for Shourya742 December 11, 2024 14:49
@codecov Codecov
Copy link

codecov bot commented Dec 11, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 19.30%. Comparing base (aeacb0e) to head (e94758b).
Report is 2 commits behind head on main.

Additional details and impacted files 
@@           Coverage Diff           @@
##             main    #1291   +/-   ##
=======================================
  Coverage   19.30%   19.30%           
=======================================
  Files         164      164           
  Lines       10849    10849           
=======================================
  Hits         2094     2094           
  Misses       8755     8755
Flag Coverage Δ
binary_codec_sv2-coverage 0.00% <ø> (ø)
binary_serde_sv2-coverage 3.65% <ø> (ø)
binary_sv2-coverage 5.48% <ø> (ø)
bip32_derivation-coverage 0.00% <ø> (ø)
buffer_sv2-coverage 25.02% <ø> (ø)
codec_sv2-coverage 0.01% <ø> (ø)
common_messages_sv2-coverage 0.13% <ø> (ø)
const_sv2-coverage 0.00% <ø> (ø)
error_handling-coverage 0.00% <ø> (ø)
framing_sv2-coverage 0.29% <ø> (ø)
jd_client-coverage 0.00% <ø> (ø)
jd_server-coverage 7.79% <ø> (ø)
job_declaration_sv2-coverage 0.00% <ø> (ø)
key-utils-coverage 2.39% <ø> (ø)
mining-coverage 2.51% <ø> (ø)
mining_device-coverage 0.00% <ø> (ø)
mining_proxy_sv2-coverage 0.70% <ø> (ø)
noise_sv2-coverage 4.35% <ø> (ø)
pool_sv2-coverage 1.38% <ø> (ø)
protocols 24.72% <ø> (ø)
roles 6.55% <ø> (ø)
roles_logic_sv2-coverage 8.08% <ø> (ø)
sv2_ffi-coverage 0.00% <ø> (ø)
template_distribution_sv2-coverage 0.00% <ø> (ø)
translator_sv2-coverage 9.60% <ø> (ø)
utils 25.13% <ø> (ø)
v1-coverage 2.47% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 11, 2024 •
edited
Loading

🐰 Bencher Report

Branchfix-tp-drop-itf
Testbedsv2
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
nanoseconds (ns)
(Result Δ%)		Upper Boundary
nanoseconds (ns)
(Limit %)
client_sv2_handle_message_common📈 view plot
🚷 view threshold		44.10
(-1.02%)		46.79
(94.26%)
client_sv2_handle_message_mining📈 view plot
🚷 view threshold		74.88
(-1.73%)		93.27
(80.28%)
client_sv2_mining_message_submit_standard📈 view plot
🚷 view threshold		14.72
(+0.37%)		14.78
(99.60%)
client_sv2_mining_message_submit_standard_serialize📈 view plot
🚷 view threshold		255.21
(-3.46%)		298.22
(85.58%)
client_sv2_mining_message_submit_standard_serialize_deserialize📈 view plot
🚷 view threshold		635.27
(+3.21%)		680.59
(93.34%)
client_sv2_open_channel📈 view plot
🚷 view threshold		165.05
(+3.45%)		192.37
(85.80%)
client_sv2_open_channel_serialize📈 view plot
🚷 view threshold		284.71
(+0.14%)		310.21
(91.78%)
client_sv2_open_channel_serialize_deserialize📈 view plot
🚷 view threshold		468.55
(+20.92%)		471.48
(99.38%)
client_sv2_setup_connection📈 view plot
🚷 view threshold		159.16
(-0.71%)		170.57
(93.31%)
client_sv2_setup_connection_serialize📈 view plot
🚷 view threshold		467.82
(-0.56%)		547.45
(85.45%)
client_sv2_setup_connection_serialize_deserialize📈 view plot
🚷 view threshold		1,043.10
(+4.86%)		1,094.32
(95.32%)
🐰 View full continuous benchmarking report in Bencher

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 11, 2024 •
edited
Loading

🐰 Bencher Report

Branchfix-tp-drop-itf
Testbedsv1
Click to view all benchmark results
BenchmarkEstimated CyclesBenchmark Result
estimated cycles
(Result Δ%)		Upper Boundary
estimated cycles
(Limit %)		InstructionsBenchmark Result
instructions
(Result Δ%)		Upper Boundary
instructions
(Limit %)		L1 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		L2 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		RAM AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)
get_authorize📈 view plot
🚷 view threshold		8,421.00
(-0.45%)		8,726.33
(96.50%)		📈 view plot
🚷 view threshold		3,689.00
(-1.40%)		3,869.10
(95.35%)		📈 view plot
🚷 view threshold		5,156.00
(-1.66%)		5,450.57
(94.60%)		📈 view plot
🚷 view threshold		9.00
(+19.75%)		14.57
(61.76%)		📈 view plot
🚷 view threshold		92.00
(+1.31%)		95.72
(96.11%)
get_submit📈 view plot
🚷 view threshold		95,205.00
(-0.21%)		95,852.64
(99.32%)		📈 view plot
🚷 view threshold		59,346.00
(-0.16%)		59,713.19
(99.39%)		📈 view plot
🚷 view threshold		85,220.00
(-0.18%)		85,817.18
(99.30%)		📈 view plot
🚷 view threshold		51.00
(+14.99%)		60.58
(84.18%)		📈 view plot
🚷 view threshold		278.00
(-0.79%)		290.12
(95.82%)
get_subscribe📈 view plot
🚷 view threshold		7,906.00
(-1.33%)		8,280.96
(95.47%)		📈 view plot
🚷 view threshold		2,765.00
(-2.20%)		2,949.09
(93.76%)		📈 view plot
🚷 view threshold		3,846.00
(-2.61%)		4,143.97
(92.81%)		📈 view plot
🚷 view threshold		14.00
(+17.82%)		20.13
(69.53%)		📈 view plot
🚷 view threshold		114.00
(-0.36%)		118.59
(96.13%)
serialize_authorize📈 view plot
🚷 view threshold		12,150.00
(-1.02%)		12,565.51
(96.69%)		📈 view plot
🚷 view threshold		5,266.00
(-0.89%)		5,435.21
(96.89%)		📈 view plot
🚷 view threshold		7,325.00
(-1.10%)		7,606.86
(96.29%)		📈 view plot
🚷 view threshold		13.00
(+38.44%)		17.49
(74.34%)		📈 view plot
🚷 view threshold		136.00
(-1.28%)		142.59
(95.38%)
serialize_deserialize_authorize📈 view plot
🚷 view threshold		24,640.00
(-0.26%)		25,206.73
(97.75%)		📈 view plot
🚷 view threshold		9,838.00
(-0.43%)		10,039.01
(98.00%)		📈 view plot
🚷 view threshold		13,875.00
(-0.52%)		14,203.60
(97.69%)		📈 view plot
🚷 view threshold		39.00
(+11.78%)		44.69
(87.27%)		📈 view plot
🚷 view threshold		302.00
(-0.12%)		314.34
(96.08%)
serialize_deserialize_handle_authorize📈 view plot
🚷 view threshold		30,118.00
(-0.68%)		30,752.46
(97.94%)		📈 view plot
🚷 view threshold		12,015.00
(-0.46%)		12,208.77
(98.41%)		📈 view plot
🚷 view threshold		16,998.00
(-0.55%)		17,312.40
(98.18%)		📈 view plot
🚷 view threshold		62.00
(+11.57%)		67.32
(92.09%)		📈 view plot
🚷 view threshold		366.00
(-1.12%)		380.95
(96.08%)
serialize_deserialize_handle_submit📈 view plot
🚷 view threshold		126,223.00
(-0.19%)		126,906.73
(99.46%)		📈 view plot
🚷 view threshold		73,200.00
(-0.10%)		73,534.22
(99.55%)		📈 view plot
🚷 view threshold		104,913.00
(-0.13%)		105,486.51
(99.46%)		📈 view plot
🚷 view threshold		111.00
(+4.67%)		126.79
(87.55%)		📈 view plot
🚷 view threshold		593.00
(-0.66%)		607.25
(97.65%)
serialize_deserialize_handle_subscribe📈 view plot
🚷 view threshold		27,714.00
(-0.56%)		28,545.26
(97.09%)		📈 view plot
🚷 view threshold		9,584.00
(-0.61%)		9,763.20
(98.16%)		📈 view plot
🚷 view threshold		13,534.00
(-0.75%)		13,831.07
(97.85%)		📈 view plot
🚷 view threshold		71.00
(+11.71%)		75.91
(93.53%)		📈 view plot
🚷 view threshold		395.00
(-0.65%)		414.77
(95.23%)
serialize_deserialize_submit📈 view plot
🚷 view threshold		115,161.00
(-0.08%)		115,781.05
(99.46%)		📈 view plot
🚷 view threshold		68,060.00
(-0.05%)		68,416.38
(99.48%)		📈 view plot
🚷 view threshold		97,646.00
(-0.07%)		98,269.02
(99.37%)		📈 view plot
🚷 view threshold		73.00
(+15.60%)		81.29
(89.80%)		📈 view plot
🚷 view threshold		490.00
(-0.41%)		500.81
(97.84%)
serialize_deserialize_subscribe📈 view plot
🚷 view threshold		23,184.00
(-0.46%)		23,929.24
(96.89%)		📈 view plot
🚷 view threshold		8,143.00
(-0.68%)		8,319.26
(97.88%)		📈 view plot
🚷 view threshold		11,449.00
(-0.84%)		11,738.50
(97.53%)		📈 view plot
🚷 view threshold		44.00
(+17.09%)		46.47
(94.69%)		📈 view plot
🚷 view threshold		329.00
(-0.37%)		345.80
(95.14%)
serialize_submit📈 view plot
🚷 view threshold		99,594.00
(-0.23%)		100,285.81
(99.31%)		📈 view plot
🚷 view threshold		61,408.00
(-0.13%)		61,742.17
(99.46%)		📈 view plot
🚷 view threshold		88,079.00
(-0.16%)		88,647.38
(99.36%)		📈 view plot
🚷 view threshold		56.00
(+19.81%)		64.40
(86.96%)		📈 view plot
🚷 view threshold		321.00
(-1.22%)		335.42
(95.70%)
serialize_subscribe📈 view plot
🚷 view threshold		11,351.00
(-0.48%)		11,674.78
(97.23%)		📈 view plot
🚷 view threshold		4,118.00
(-1.38%)		4,291.00
(95.97%)		📈 view plot
🚷 view threshold		5,706.00
(-1.73%)		5,996.68
(95.15%)		📈 view plot
🚷 view threshold		16.00
(+23.00%)		21.71
(73.70%)		📈 view plot
🚷 view threshold		159.00
(+0.55%)		163.75
(97.10%)
🐰 View full continuous benchmarking report in Bencher

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 11, 2024 •
edited
Loading

🐰 Bencher Report

Branchfix-tp-drop-itf
Testbedsv1
Click to view all benchmark results
BenchmarkLatencyBenchmark Result
nanoseconds (ns)
(Result Δ%)		Upper Boundary
nanoseconds (ns)
(Limit %)
client-submit-serialize📈 view plot
🚷 view threshold		6,617.70
(-0.46%)		7,105.24
(93.14%)
client-submit-serialize-deserialize📈 view plot
🚷 view threshold		7,470.70
(-0.89%)		8,116.32
(92.05%)
client-submit-serialize-deserialize-handle/client-submit-serialize-deserialize-handle📈 view plot
🚷 view threshold		8,017.20
(-1.34%)		8,614.82
(93.06%)
client-sv1-authorize-serialize-deserialize-handle/client-sv1-authorize-serialize-deserialize-handle📈 view plot
🚷 view threshold		854.03
(-1.23%)		922.44
(92.58%)
client-sv1-authorize-serialize-deserialize/client-sv1-authorize-serialize-deserialize📈 view plot
🚷 view threshold		669.52
(-0.50%)		708.90
(94.44%)
client-sv1-authorize-serialize/client-sv1-authorize-serialize📈 view plot
🚷 view threshold		247.13
(-0.62%)		269.14
(91.82%)
client-sv1-get-authorize/client-sv1-get-authorize📈 view plot
🚷 view threshold		156.21
(-0.59%)		164.42
(95.00%)
client-sv1-get-submit📈 view plot
🚷 view threshold		6,460.40
(+0.30%)		7,067.67
(91.41%)
client-sv1-get-subscribe/client-sv1-get-subscribe📈 view plot
🚷 view threshold		280.72
(+0.38%)		302.13
(92.91%)
client-sv1-subscribe-serialize-deserialize-handle/client-sv1-subscribe-serialize-deserialize-handle📈 view plot
🚷 view threshold		729.35
(+0.13%)		776.85
(93.89%)
client-sv1-subscribe-serialize-deserialize/client-sv1-subscribe-serialize-deserialize📈 view plot
🚷 view threshold		595.72
(+0.83%)		632.39
(94.20%)
client-sv1-subscribe-serialize/client-sv1-subscribe-serialize📈 view plot
🚷 view threshold		203.30
(-1.29%)		218.49
(93.05%)
🐰 View full continuous benchmarking report in Bencher

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Dec 11, 2024 •
edited
Loading

🐰 Bencher Report

Branchfix-tp-drop-itf
Testbedsv2

🚨 4 Alerts

BenchmarkMeasure
Units		ViewBenchmark Result
(Result Δ%)		Upper Boundary
(Limit %)
client_sv2_handle_message_miningEstimated Cycles
estimated cycles		📈 plot
🚨 alert (🔔)
🚷 threshold		8,360.00
(+1.81%)		8,345.03
(100.18%)
client_sv2_handle_message_miningRAM Accesses
accesses		📈 plot
🚨 alert (🔔)
🚷 threshold		143.00
(+2.68%)		142.91
(100.06%)
client_sv2_mining_message_submit_standardEstimated Cycles
estimated cycles		📈 plot
🚨 alert (🔔)
🚷 threshold		6,433.00
(+2.26%)		6,431.78
(100.02%)
client_sv2_mining_message_submit_standard_serialize_deserializeL2 Accesses
accesses		📈 plot
🚨 alert (🔔)
🚷 threshold		94.00
(+14.45%)		93.91
(100.10%)
Click to view all benchmark results
BenchmarkEstimated CyclesBenchmark Result
estimated cycles
(Result Δ%)		Upper Boundary
estimated cycles
(Limit %)		InstructionsBenchmark Result
instructions
(Result Δ%)		Upper Boundary
instructions
(Limit %)		L1 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		L2 AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)		RAM AccessesBenchmark Result
accesses
(Result Δ%)		Upper Boundary
accesses
(Limit %)
client_sv2_handle_message_common📈 view plot
🚷 view threshold		2,163.00
(+2.88%)		2,218.26
(97.51%)		📈 view plot
🚷 view threshold		473.00
(-0.09%)		488.67
(96.79%)		📈 view plot
🚷 view threshold		733.00
(-0.41%)		757.34
(96.79%)		📈 view plot
🚷 view threshold		6.00
(+16.19%)		13.17
(45.56%)		📈 view plot
🚷 view threshold		40.00
(+4.43%)		41.45
(96.50%)
client_sv2_handle_message_mining📈 view plot
🚨 view alert (🔔)
🚷 view threshold		8,360.00
(+1.81%)		8,345.03
(100.18%)		📈 view plot
🚷 view threshold		2,137.00
(-0.03%)		2,143.08
(99.72%)		📈 view plot
🚷 view threshold		3,150.00
(-0.33%)		3,171.32
(99.33%)		📈 view plot
🚷 view threshold		41.00
(+15.88%)		41.51
(98.78%)		📈 view plot
🚨 view alert (🔔)
🚷 view threshold		143.00
(+2.68%)		142.91
(100.06%)
client_sv2_mining_message_submit_standard📈 view plot
🚨 view alert (🔔)
🚷 view threshold		6,433.00
(+2.26%)		6,431.78
(100.02%)		📈 view plot
🚷 view threshold		1,750.00
(-0.06%)		1,766.99
(99.04%)		📈 view plot
🚷 view threshold		2,543.00
(-0.40%)		2,575.66
(98.73%)		📈 view plot
🚷 view threshold		22.00
(+29.41%)		24.40
(90.16%)		📈 view plot
🚷 view threshold		108.00
(+3.48%)		108.25
(99.77%)
client_sv2_mining_message_submit_standard_serialize📈 view plot
🚷 view threshold		14,896.00
(+1.16%)		14,945.96
(99.67%)		📈 view plot
🚷 view threshold		4,694.00
(-0.02%)		4,710.99
(99.64%)		📈 view plot
🚷 view threshold		6,741.00
(-0.25%)		6,783.82
(99.37%)		📈 view plot
🚷 view threshold		56.00
(+27.14%)		56.99
(98.26%)		📈 view plot
🚷 view threshold		225.00
(+1.64%)		226.96
(99.14%)
client_sv2_mining_message_submit_standard_serialize_deserialize📈 view plot
🚷 view threshold		27,868.00
(+1.13%)		27,922.20
(99.81%)		📈 view plot
🚷 view threshold		10,645.00
(+0.48%)		10,673.21
(99.74%)		📈 view plot
🚷 view threshold		15,498.00
(+0.52%)		15,549.47
(99.67%)		📈 view plot
🚨 view alert (🔔)
🚷 view threshold		94.00
(+14.45%)		93.91
(100.10%)		📈 view plot
🚷 view threshold		340.00
(+1.46%)		341.88
(99.45%)
client_sv2_open_channel📈 view plot
🚷 view threshold		4,519.00
(+2.97%)		4,564.97
(98.99%)		📈 view plot
🚷 view threshold		1,461.00
(-0.03%)		1,476.67
(98.94%)		📈 view plot
🚷 view threshold		2,154.00
(-0.30%)		2,183.39
(98.65%)		📈 view plot
🚷 view threshold		11.00
(+29.89%)		15.52
(70.86%)		📈 view plot
🚷 view threshold		66.00
(+5.68%)		67.33
(98.02%)
client_sv2_open_channel_serialize📈 view plot
🚷 view threshold		14,154.00
(+0.90%)		14,272.43
(99.17%)		📈 view plot
🚷 view threshold		5,064.00
(-0.01%)		5,079.67
(99.69%)		📈 view plot
🚷 view threshold		7,314.00
(-0.17%)		7,350.64
(99.50%)		📈 view plot
🚷 view threshold		45.00
(+27.24%)		45.04
(99.92%)		📈 view plot
🚷 view threshold		189.00
(+1.38%)		193.50
(97.68%)
client_sv2_open_channel_serialize_deserialize📈 view plot
🚷 view threshold		22,888.00
(+0.96%)		22,946.38
(99.75%)		📈 view plot
🚷 view threshold		8,040.00
(+0.16%)		8,064.89
(99.69%)		📈 view plot
🚷 view threshold		11,688.00
(+0.08%)		11,733.51
(99.61%)		📈 view plot
🚷 view threshold		84.00
(+12.40%)		86.08
(97.58%)		📈 view plot
🚷 view threshold		308.00
(+1.53%)		310.46
(99.21%)
client_sv2_setup_connection📈 view plot
🚷 view threshold		4,749.00
(+1.33%)		4,783.51
(99.28%)		📈 view plot
🚷 view threshold		1,502.00
(-0.03%)		1,517.67
(98.97%)		📈 view plot
🚷 view threshold		2,274.00
(-0.19%)		2,299.22
(98.90%)		📈 view plot
🚷 view threshold		12.00
(+24.17%)		16.54
(72.53%)		📈 view plot
🚷 view threshold		69.00
(+2.34%)		70.03
(98.53%)
client_sv2_setup_connection_serialize📈 view plot
🚷 view threshold		16,282.00
(+0.78%)		16,375.12
(99.43%)		📈 view plot
🚷 view threshold		5,963.00
(-0.01%)		5,978.67
(99.74%)		📈 view plot
🚷 view threshold		8,652.00
(-0.14%)		8,690.59
(99.56%)		📈 view plot
🚷 view threshold		49.00
(+22.93%)		53.18
(92.15%)		📈 view plot
🚷 view threshold		211.00
(+1.28%)		213.99
(98.60%)
client_sv2_setup_connection_serialize_deserialize📈 view plot
🚷 view threshold		35,782.00
(+0.61%)		35,838.83
(99.84%)		📈 view plot
🚷 view threshold		14,888.00
(+0.20%)		14,911.89
(99.84%)		📈 view plot
🚷 view threshold		21,867.00
(+0.17%)		21,919.32
(99.76%)		📈 view plot
🚷 view threshold		109.00
(+18.01%)		112.58
(96.82%)		📈 view plot
🚷 view threshold		382.00
(+0.72%)		384.89
(99.25%)
🐰 View full continuous benchmarking report in Bencher

Shourya742
Shourya742 approved these changes Dec 11, 2024
View reviewed changes
Copy link
Contributor

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

Damn... That's nice catch, for me its working fine now locally. ACK

@GitGab19 GitGab19 added the ready-to-be-merged triggers auto rebase bot label Dec 11, 2024
GitGab19
GitGab19 approved these changes Dec 11, 2024
View reviewed changes
Copy link
Collaborator

@GitGab19 GitGab19 left a comment

Choose a reason for hiding this comment

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

tACK

@GitGab19 GitGab19 mentioned this pull request Dec 11, 2024
Closed
@GitGab19 GitGab19 merged commit 22da418 into stratum-mining:main Dec 11, 2024
38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Shourya742 Shourya742 Shourya742 approved these changes

@GitGab19 GitGab19 GitGab19 approved these changes

@jbesraa jbesraa Awaiting requested review from jbesraa

Assignees
No one assigned
Labels
ready-to-be-merged triggers auto rebase bot
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

need to fix TP dropping logic on ITF
3 participants
@plebhash @GitGab19 @Shourya742