pezkuwichain/pezkuwi-subxt
1
0
Fork 0
mirror of https://github.com/pezkuwichain/pezkuwi-subxt.git synced 2026-06-11 11:51:12 +00:00
Code Issues Packages Projects Releases Wiki Activity

Issue 4393: Correcting Unnecessary Use of Arc (#6882)

Browse Source 
* Added participation and queue sizes metrics

* First draft of all metric code

* Tests pass

* Changed Metrics to field on participation + queues

* fmt

* Improving naming

* Refactor, placing timer in ParticipationRequest

* fmt

* Final cleanup

* Revert "Final cleanup"

This reverts commit 02e5608df64b2e0f7810905e4508673b2037d351.

* Changing metric names

* Implementing Eq only for unit tests

* fmt

* Removing Clone trait from ParticipationRequest

* fmt

* Moved clone functionality to tests helper
This commit is contained in:
Bradley Olson
2023-03-16 02:00:56 -07:00
committed by GitHub
parent e1e46c26cb
commit 200e9dfa29
5 changed files with 35 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
polkadot/node/core/dispute-coordinator/src/initialized.rs
+1 -1
View File
@@ -916,7 +916,7 @@ impl Initialized {
} else {
self.metrics.on_queued_best_effort_participation();
}
let request_timer = Arc::new(self.metrics.time_participation_pipeline());
let request_timer = self.metrics.time_participation_pipeline();
let r = self
.participation
.queue_participation(
polkadot/node/core/dispute-coordinator/src/lib.rs
+1 -1
View File
@@ -347,7 +347,7 @@ impl DisputeCoordinatorSubsystem {
?candidate_hash,
"Found valid dispute, with no vote from us on startup - participating."
);
let request_timer = Arc::new(self.metrics.time_participation_pipeline());
let request_timer = self.metrics.time_participation_pipeline();
participation_requests.push((
ParticipationPriority::with_priority_if(is_included),
ParticipationRequest::new(
polkadot/node/core/dispute-coordinator/src/participation/queues/mod.rs
+6 -6
View File
@@ -14,7 +14,7 @@
// You should have received a copy of the GNU General Public License
// along with Polkadot. If not, see <http://www.gnu.org/licenses/>.
use std::{cmp::Ordering, collections::BTreeMap, sync::Arc};
use std::{cmp::Ordering, collections::BTreeMap};
use futures::channel::oneshot;
use polkadot_node_subsystem::{messages::ChainApiMessage, overseer};
@@ -65,12 +65,12 @@ pub struct Queues {
}
/// A dispute participation request that can be queued.
#[derive(Debug, Clone)]
#[derive(Debug)]
pub struct ParticipationRequest {
candidate_hash: CandidateHash,
candidate_receipt: CandidateReceipt,
session: SessionIndex,
_request_timer: Arc<Option<prometheus::HistogramTimer>>, // Sends metric data when request is dropped
_request_timer: Option<prometheus::HistogramTimer>, // Sends metric data when request is dropped
}
/// Whether a `ParticipationRequest` should be put on best-effort or the priority queue.
@@ -117,7 +117,7 @@ impl ParticipationRequest {
pub fn new(
candidate_receipt: CandidateReceipt,
session: SessionIndex,
request_timer: Arc<Option<prometheus::HistogramTimer>>,
request_timer: Option<prometheus::HistogramTimer>,
) -> Self {
Self {
candidate_hash: candidate_receipt.hash(),
@@ -142,8 +142,8 @@ impl ParticipationRequest {
}
}
// We want to compare participation requests in unit tests, so we
// only implement Eq for tests.
// We want to compare and clone participation requests in unit tests, so we
// only implement Eq and Clone for tests.
#[cfg(test)]
impl PartialEq for ParticipationRequest {
fn eq(&self, other: &Self) -> bool {
polkadot/node/core/dispute-coordinator/src/participation/queues/tests.rs
+26 -15
View File
@@ -18,7 +18,6 @@ use crate::{metrics::Metrics, ParticipationPriority};
use ::test_helpers::{dummy_candidate_receipt, dummy_hash};
use assert_matches::assert_matches;
use polkadot_primitives::{BlockNumber, Hash};
use std::sync::Arc;
use super::{CandidateComparator, ParticipationRequest, QueueError, Queues};
@@ -27,7 +26,7 @@ fn make_participation_request(hash: Hash) -> ParticipationRequest {
let mut receipt = dummy_candidate_receipt(dummy_hash());
// make it differ:
receipt.commitments_hash = hash;
let request_timer = Arc::new(Metrics::default().time_participation_pipeline());
let request_timer = Metrics::default().time_participation_pipeline();
ParticipationRequest::new(receipt, 1, request_timer)
}
@@ -39,6 +38,18 @@ fn make_dummy_comparator(
CandidateComparator::new_dummy(relay_parent, *req.candidate_hash())
}
/// Make a partial clone of the given ParticipationRequest, just missing
/// the request_timer field. We prefer this helper to implementing Clone
/// for ParticipationRequest, since we only clone requests in tests.
fn clone_request(request: &ParticipationRequest) -> ParticipationRequest {
ParticipationRequest {
candidate_receipt: request.candidate_receipt.clone(),
candidate_hash: request.candidate_hash.clone(),
session: request.session,
_request_timer: None,
}
}
/// Check that dequeuing acknowledges order.
///
/// Any priority item will be dequeued before any best effort items, priority and best effort with
@@ -59,35 +70,35 @@ fn ordering_works_as_expected() {
.queue_with_comparator(
make_dummy_comparator(&req1, Some(1)),
ParticipationPriority::BestEffort,
req1.clone(),
clone_request(&req1),
)
.unwrap();
queue
.queue_with_comparator(
make_dummy_comparator(&req_prio, Some(1)),
ParticipationPriority::Priority,
req_prio.clone(),
clone_request(&req_prio),
)
.unwrap();
queue
.queue_with_comparator(
make_dummy_comparator(&req3, Some(2)),
ParticipationPriority::BestEffort,
req3.clone(),
clone_request(&req3),
)
.unwrap();
queue
.queue_with_comparator(
make_dummy_comparator(&req_prio_2, Some(2)),
ParticipationPriority::Priority,
req_prio_2.clone(),
clone_request(&req_prio_2),
)
.unwrap();
queue
.queue_with_comparator(
make_dummy_comparator(&req5_unknown_parent, None),
ParticipationPriority::BestEffort,
req5_unknown_parent.clone(),
clone_request(&req5_unknown_parent),
)
.unwrap();
assert_matches!(
@@ -132,14 +143,14 @@ fn candidate_is_only_dequeued_once() {
.queue_with_comparator(
make_dummy_comparator(&req1, None),
ParticipationPriority::BestEffort,
req1.clone(),
clone_request(&req1),
)
.unwrap();
queue
.queue_with_comparator(
make_dummy_comparator(&req_prio, Some(1)),
ParticipationPriority::Priority,
req_prio.clone(),
clone_request(&req_prio),
)
.unwrap();
// Insert same best effort again:
@@ -147,7 +158,7 @@ fn candidate_is_only_dequeued_once() {
.queue_with_comparator(
make_dummy_comparator(&req1, None),
ParticipationPriority::BestEffort,
req1.clone(),
clone_request(&req1),
)
.unwrap();
// insert same prio again:
@@ -155,7 +166,7 @@ fn candidate_is_only_dequeued_once() {
.queue_with_comparator(
make_dummy_comparator(&req_prio, Some(1)),
ParticipationPriority::Priority,
req_prio.clone(),
clone_request(&req_prio),
)
.unwrap();
// Insert first as best effort:
@@ -163,7 +174,7 @@ fn candidate_is_only_dequeued_once() {
.queue_with_comparator(
make_dummy_comparator(&req_best_effort_then_prio, Some(2)),
ParticipationPriority::BestEffort,
req_best_effort_then_prio.clone(),
clone_request(&req_best_effort_then_prio),
)
.unwrap();
// Then as prio:
@@ -171,7 +182,7 @@ fn candidate_is_only_dequeued_once() {
.queue_with_comparator(
make_dummy_comparator(&req_best_effort_then_prio, Some(2)),
ParticipationPriority::Priority,
req_best_effort_then_prio.clone(),
clone_request(&req_best_effort_then_prio),
)
.unwrap();
@@ -183,7 +194,7 @@ fn candidate_is_only_dequeued_once() {
.queue_with_comparator(
make_dummy_comparator(&req_prio_then_best_effort, Some(3)),
ParticipationPriority::Priority,
req_prio_then_best_effort.clone(),
clone_request(&req_prio_then_best_effort),
)
.unwrap();
// Then as best effort:
@@ -191,7 +202,7 @@ fn candidate_is_only_dequeued_once() {
.queue_with_comparator(
make_dummy_comparator(&req_prio_then_best_effort, Some(3)),
ParticipationPriority::BestEffort,
req_prio_then_best_effort.clone(),
clone_request(&req_prio_then_best_effort),
)
.unwrap();
polkadot/node/core/dispute-coordinator/src/participation/tests.rs
+1 -1
View File
@@ -72,7 +72,7 @@ async fn participate_with_commitments_hash<Context>(
};
let session = 1;
let request_timer = Arc::new(participation.metrics.time_participation_pipeline());
let request_timer = participation.metrics.time_participation_pipeline();
let req = ParticipationRequest::new(candidate_receipt, session, request_timer);
participation