From 6c251c3e3891df4fc52d54315686609ba60ceaac Mon Sep 17 00:00:00 2001 From: Peter Goodspeed-Niklaus Date: Wed, 17 Mar 2021 09:13:33 +0100 Subject: [PATCH] Improve complexity of CompactAssignments::unique_targets (#8314) * Improve complexity of CompactAssignments::unique_targets Original implementation was O(n**2). Current impl is O(n log n). Avoided the original proposed mitigation because it does not retain the de-duplicating property present in the original implementation. This implementation does a little more work, but retains that property. * Explicitly choose sp_std Vec and BTreeSet Ensures that the macro still works if someone uses it in a context in which sp_std is not imported or is renamed. * explicitly use sp_std vectors throughout compact macro --- .../npos-elections/compact/src/assignment.rs | 2 +- .../npos-elections/compact/src/codec.rs | 20 +++++++-------- primitives/npos-elections/compact/src/lib.rs | 25 +++++++++---------- primitives/npos-elections/src/lib.rs | 2 ++ 4 files changed, 25 insertions(+), 24 deletions(-) diff --git a/primitives/npos-elections/compact/src/assignment.rs b/primitives/npos-elections/compact/src/assignment.rs index 12f5ca2b41735..2c8edefbfb379 100644 --- a/primitives/npos-elections/compact/src/assignment.rs +++ b/primitives/npos-elections/compact/src/assignment.rs @@ -125,7 +125,7 @@ pub(crate) fn into_impl(count: usize, per_thing: syn::Type) -> TokenStream2 { let target = target_at(*t_idx).or_invalid_index()?; Ok((target, *p)) }) - .collect::, _npos::Error>>()?; + .collect::, _npos::Error>>()?; if sum >= #per_thing::one() { return Err(_npos::Error::CompactStakeOverflow); diff --git a/primitives/npos-elections/compact/src/codec.rs b/primitives/npos-elections/compact/src/codec.rs index 6e8d4d9277dbd..f75f99682711c 100644 --- a/primitives/npos-elections/compact/src/codec.rs +++ b/primitives/npos-elections/compact/src/codec.rs @@ -49,14 +49,14 @@ fn decode_impl( quote! { let #name = < - Vec<(_npos::codec::Compact<#voter_type>, _npos::codec::Compact<#target_type>)> + _npos::sp_std::prelude::Vec<(_npos::codec::Compact<#voter_type>, _npos::codec::Compact<#target_type>)> as _npos::codec::Decode >::decode(value)?; let #name = #name .into_iter() .map(|(v, t)| (v.0, t.0)) - .collect::>(); + .collect::<_npos::sp_std::prelude::Vec<_>>(); } }; @@ -65,7 +65,7 @@ fn decode_impl( quote! { let #name = < - Vec<( + _npos::sp_std::prelude::Vec<( _npos::codec::Compact<#voter_type>, (_npos::codec::Compact<#target_type>, _npos::codec::Compact<#weight_type>), _npos::codec::Compact<#target_type>, @@ -76,7 +76,7 @@ fn decode_impl( let #name = #name .into_iter() .map(|(v, (t1, w), t2)| (v.0, (t1.0, w.0), t2.0)) - .collect::>(); + .collect::<_npos::sp_std::prelude::Vec<_>>(); } }; @@ -90,7 +90,7 @@ fn decode_impl( quote! { let #name = < - Vec<( + _npos::sp_std::prelude::Vec<( _npos::codec::Compact<#voter_type>, [(_npos::codec::Compact<#target_type>, _npos::codec::Compact<#weight_type>); #c-1], _npos::codec::Compact<#target_type>, @@ -104,7 +104,7 @@ fn decode_impl( [ #inner_impl ], t_last.0, )) - .collect::>(); + .collect::<_npos::sp_std::prelude::Vec<_>>(); } }).collect::(); @@ -142,7 +142,7 @@ fn encode_impl(ident: syn::Ident, count: usize) -> TokenStream2 { _npos::codec::Compact(v.clone()), _npos::codec::Compact(t.clone()), )) - .collect::>(); + .collect::<_npos::sp_std::prelude::Vec<_>>(); #name.encode_to(&mut r); } }; @@ -160,7 +160,7 @@ fn encode_impl(ident: syn::Ident, count: usize) -> TokenStream2 { ), _npos::codec::Compact(t2.clone()), )) - .collect::>(); + .collect::<_npos::sp_std::prelude::Vec<_>>(); #name.encode_to(&mut r); } }; @@ -184,14 +184,14 @@ fn encode_impl(ident: syn::Ident, count: usize) -> TokenStream2 { [ #inners_compact_array ], _npos::codec::Compact(t_last.clone()), )) - .collect::>(); + .collect::<_npos::sp_std::prelude::Vec<_>>(); #name.encode_to(&mut r); } }).collect::(); quote!( impl _npos::codec::Encode for #ident { - fn encode(&self) -> Vec { + fn encode(&self) -> _npos::sp_std::prelude::Vec { let mut r = vec![]; #encode_impl_single #encode_impl_double diff --git a/primitives/npos-elections/compact/src/lib.rs b/primitives/npos-elections/compact/src/lib.rs index ed1837bae18b1..dd6d4de9b0241 100644 --- a/primitives/npos-elections/compact/src/lib.rs +++ b/primitives/npos-elections/compact/src/lib.rs @@ -119,14 +119,14 @@ fn struct_def( let name = field_name_for(1); // NOTE: we use the visibility of the struct for the fields as well.. could be made better. quote!( - #vis #name: Vec<(#voter_type, #target_type)>, + #vis #name: _npos::sp_std::prelude::Vec<(#voter_type, #target_type)>, ) }; let doubles = { let name = field_name_for(2); quote!( - #vis #name: Vec<(#voter_type, (#target_type, #weight_type), #target_type)>, + #vis #name: _npos::sp_std::prelude::Vec<(#voter_type, (#target_type, #weight_type), #target_type)>, ) }; @@ -135,7 +135,7 @@ fn struct_def( let field_name = field_name_for(c); let array_len = c - 1; quote!( - #vis #field_name: Vec<( + #vis #field_name: _npos::sp_std::prelude::Vec<( #voter_type, [(#target_type, #weight_type); #array_len], #target_type @@ -194,20 +194,19 @@ fn struct_def( all_edges } - fn unique_targets(&self) -> Vec { + fn unique_targets(&self) -> _npos::sp_std::prelude::Vec { // NOTE: this implementation returns the targets sorted, but we don't use it yet per // se, nor is the API enforcing it. - let mut all_targets: Vec = Vec::with_capacity(self.average_edge_count()); + use _npos::sp_std::collections::btree_set::BTreeSet; + + let mut all_targets: BTreeSet = BTreeSet::new(); let mut maybe_insert_target = |t: Self::Target| { - match all_targets.binary_search(&t) { - Ok(_) => (), - Err(pos) => all_targets.insert(pos, t) - } + all_targets.insert(t); }; #unique_targets_impl - all_targets + all_targets.into_iter().collect() } fn remove_voter(&mut self, to_remove: Self::Voter) -> bool { @@ -216,7 +215,7 @@ fn struct_def( } fn from_assignment( - assignments: Vec<_npos::Assignment>, + assignments: _npos::sp_std::prelude::Vec<_npos::Assignment>, index_of_voter: FV, index_of_target: FT, ) -> Result @@ -243,8 +242,8 @@ fn struct_def( self, voter_at: impl Fn(Self::Voter) -> Option, target_at: impl Fn(Self::Target) -> Option, - ) -> Result>, _npos::Error> { - let mut assignments: Vec<_npos::Assignment> = Default::default(); + ) -> Result<_npos::sp_std::prelude::Vec<_npos::Assignment>, _npos::Error> { + let mut assignments: _npos::sp_std::prelude::Vec<_npos::Assignment> = Default::default(); #into_impl Ok(assignments) } diff --git a/primitives/npos-elections/src/lib.rs b/primitives/npos-elections/src/lib.rs index 10ee12cc55085..05505d06f201e 100644 --- a/primitives/npos-elections/src/lib.rs +++ b/primitives/npos-elections/src/lib.rs @@ -119,6 +119,8 @@ pub use pjr::*; pub use codec; #[doc(hidden)] pub use sp_arithmetic; +#[doc(hidden)] +pub use sp_std; /// Simple Extension trait to easily convert `None` from index closures to `Err`. ///