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

Fix the performance issue caused by track_entry_statics #4382

Merged
merged 4 commits into from
Mar 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions test/src/specs/rpc/truncate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ impl Spec for RpcTruncate {

let tx_pool_info = node.get_tip_tx_pool_info();
assert!(tx_pool_info.total_tx_size.value() > 0, "tx-pool holds tx2");
assert!(tx_pool_info.pending.value() > 0, "tx-pool hods tx2");

// Truncate from `to_truncate`

Expand Down
53 changes: 36 additions & 17 deletions tx-pool/src/component/pool_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ use ckb_types::{
};
use multi_index_map::MultiIndexMap;
use std::collections::HashSet;

type ConflictEntry = (TxEntry, Reject);

#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, Hash)]
Expand Down Expand Up @@ -71,6 +70,9 @@ pub struct PoolMap {
pub(crate) total_tx_size: usize,
// sum of all tx_pool tx's cycles.
pub(crate) total_tx_cycles: Cycle,
pub(crate) pending_count: usize,
pub(crate) gap_count: usize,
pub(crate) proposed_count: usize,
}

impl PoolMap {
Expand All @@ -82,6 +84,9 @@ impl PoolMap {
max_ancestors_count,
total_tx_size: 0,
total_tx_cycles: 0,
pending_count: 0,
gap_count: 0,
proposed_count: 0,
}
}

Expand Down Expand Up @@ -139,17 +144,12 @@ impl PoolMap {
self.get_by_id(id).expect("unconsistent pool")
}

pub(crate) fn get_by_status(&self, status: Status) -> Vec<&PoolEntry> {
self.entries.get_by_status(&status)
}

pub(crate) fn pending_size(&self) -> usize {
self.entries.get_by_status(&Status::Pending).len()
+ self.entries.get_by_status(&Status::Gap).len()
self.pending_count + self.gap_count
}

pub(crate) fn proposed_size(&self) -> usize {
self.entries.get_by_status(&Status::Proposed).len()
self.proposed_count
}

pub(crate) fn sorted_proposed_iter(&self) -> impl Iterator<Item = &TxEntry> {
Expand Down Expand Up @@ -213,19 +213,21 @@ impl PoolMap {
self.record_entry_edges(&entry)?;
self.insert_entry(&entry, status);
self.record_entry_descendants(&entry);
self.track_entry_statics();
self.track_entry_statics(None, Some(status));
self.update_stat_for_add_tx(entry.size, entry.cycles);
Ok((true, evicts))
}

/// Change the status of the entry, only used for `gap_rtx` and `proposed_rtx`
pub(crate) fn set_entry(&mut self, short_id: &ProposalShortId, status: Status) {
let mut old_status = None;
self.entries
.modify_by_id(short_id, |e| {
old_status = Some(e.status);
e.status = status;
})
.expect("unconsistent pool");
self.track_entry_statics();
self.track_entry_statics(old_status, Some(status));
}

pub(crate) fn remove_entry(&mut self, id: &ProposalShortId) -> Option<TxEntry> {
Expand All @@ -239,6 +241,7 @@ impl PoolMap {
self.update_descendants_index_key(&entry.inner, EntryOp::Remove);
self.remove_entry_edges(&entry.inner);
self.remove_entry_links(id);
self.track_entry_statics(Some(entry.status), None);
self.update_stat_for_remove_tx(entry.inner.size, entry.inner.cycles);
entry.inner
})
Expand Down Expand Up @@ -362,6 +365,9 @@ impl PoolMap {
self.links.clear();
self.total_tx_size = 0;
self.total_tx_cycles = 0;
self.pending_count = 0;
self.gap_count = 0;
self.proposed_count = 0;
}

pub(crate) fn score_sorted_iter_by(
Expand Down Expand Up @@ -625,20 +631,33 @@ impl PoolMap {
});
}

fn track_entry_statics(&self) {
fn track_entry_statics(&mut self, remove: Option<Status>, add: Option<Status>) {
match remove {
Some(Status::Pending) => self.pending_count -= 1,
Some(Status::Gap) => self.gap_count -= 1,
Some(Status::Proposed) => self.proposed_count -= 1,
_ => {}
}
match add {
Some(Status::Pending) => self.pending_count += 1,
Some(Status::Gap) => self.gap_count += 1,
Some(Status::Proposed) => self.proposed_count += 1,
_ => {}
}
assert_eq!(
self.pending_count + self.gap_count + self.proposed_count,
self.entries.len()
);
if let Some(metrics) = ckb_metrics::handle() {
metrics
.ckb_tx_pool_entry
.pending
.set(self.entries.get_by_status(&Status::Pending).len() as i64);
metrics
.ckb_tx_pool_entry
.gap
.set(self.entries.get_by_status(&Status::Gap).len() as i64);
.set(self.pending_count as i64);
metrics.ckb_tx_pool_entry.gap.set(self.gap_count as i64);
metrics
.ckb_tx_pool_entry
.proposed
.set(self.proposed_size() as i64);
.set(self.proposed_count as i64);
}
}

Expand Down
66 changes: 66 additions & 0 deletions tx-pool/src/component/tests/proposed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@ use crate::component::tests::util::{
build_tx, build_tx_with_dep, build_tx_with_header_dep, DEFAULT_MAX_ANCESTORS_COUNT,
MOCK_CYCLES, MOCK_FEE, MOCK_SIZE,
};
use ckb_types::core::{capacity_bytes, ScriptHashType};
use ckb_types::packed::{CellOutputBuilder, ScriptBuilder};
use ckb_types::H256;
use std::time::Instant;

use crate::component::{entry::TxEntry, pool_map::PoolMap};
use ckb_types::{
Expand Down Expand Up @@ -732,3 +736,65 @@ fn test_container_bench_add_limits() {
pool.clear();
assert_eq!(pool.size(), 0);
}

#[test]
fn test_pool_map_bench() {
use rand::Rng;
let mut rng = rand::thread_rng();

let mut pool = PoolMap::new(150);

let mut instant = Instant::now();
let mut time_spend = vec![];
for i in 0..20000 {
let lock_script1 = ScriptBuilder::default()
.code_hash(H256(rand::random()).pack())
.hash_type(ScriptHashType::Data.into())
.args(Bytes::from(b"lock_script1".to_vec()).pack())
.build();

let type_script1 = ScriptBuilder::default()
.code_hash(H256(rand::random()).pack())
.hash_type(ScriptHashType::Data.into())
.args(Bytes::from(b"type_script1".to_vec()).pack())
.build();

let tx = TransactionBuilder::default()
.output(
CellOutputBuilder::default()
.capacity(capacity_bytes!(1000).pack())
.lock(lock_script1)
.type_(Some(type_script1).pack())
.build(),
)
.output_data(Default::default())
.build();

let entry = TxEntry::dummy_resolve(
tx,
rng.gen_range(0..1000),
Capacity::shannons(200),
rng.gen_range(0..1000),
);
if i % 5000 == 0 && i != 0 {
eprintln!("i: {}, time: {:?}", i, instant.elapsed());
time_spend.push(instant.elapsed());
instant = Instant::now();
}
let status = if rng.gen_range(0..100) >= 30 {
Status::Pending
} else {
Status::Gap
};
let _ = pool.add_entry(entry, status);
}
let first = time_spend[0].as_millis();
let last = time_spend.last().unwrap().as_millis();
let diff = (last as i128 - first as i128).abs();
let expect_diff_range = ((first as f64) * 2.0) as i128;
eprintln!(
"first: {} last: {}, diff: {}, range: {}",
first, last, diff, expect_diff_range
);
assert!(diff < expect_diff_range);
}
10 changes: 1 addition & 9 deletions tx-pool/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,15 +72,6 @@ impl TxPool {
Arc::clone(&self.snapshot)
}

fn get_by_status(&self, status: Status) -> Vec<&PoolEntry> {
self.pool_map.get_by_status(status)
}

/// Get tx-pool size
pub fn status_size(&self, status: Status) -> usize {
self.get_by_status(status).len()
}

/// Check whether tx-pool enable RBF
pub fn enable_rbf(&self) -> bool {
self.config.min_rbf_rate > self.config.min_fee_rate
Expand Down Expand Up @@ -254,6 +245,7 @@ impl TxPool {
// Expire all transaction (and their dependencies) in the pool.
pub(crate) fn remove_expired(&mut self, callbacks: &Callbacks) {
let now_ms = ckb_systemtime::unix_time_as_millis();

let removed: Vec<_> = self
.pool_map
.iter()
Expand Down
Loading