Skip to content

Commit 7ee4bbb

Browse files
tnulltnull
committed
Drop a bunch of unnecessary Arc::clones
Previously, we consistently handed around `Arc` references for most objects to avoid unnecessary refactoring work. This approach however introduced a bunch of unnecessary allocations through `Arc::clone`. Here we opt to rather use plain references in a bunch of places, reducing the usage of `Arc`s.
1 parent 0fb8877 commit 7ee4bbb

File tree

9 files changed

+85
-141
lines changed

9 files changed

+85
-141
lines changed

‎src/builder.rs‎

Lines changed: 23 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -1053,7 +1053,7 @@ fn build_with_store_internal(
10531053

10541054
// Initialize the status fields.
10551055
let node_metrics = match runtime
1056-
.block_on(async { read_node_metrics(Arc::clone(&kv_store), Arc::clone(&logger)).await })
1056+
.block_on(async { read_node_metrics(&*kv_store, Arc::clone(&logger)).await })
10571057
{
10581058
Ok(metrics) => Arc::new(RwLock::new(metrics)),
10591059
Err(e) => {
@@ -1068,21 +1068,20 @@ fn build_with_store_internal(
10681068
let tx_broadcaster = Arc::new(TransactionBroadcaster::new(Arc::clone(&logger)));
10691069
let fee_estimator = Arc::new(OnchainFeeEstimator::new());
10701070

1071-
let payment_store = match runtime
1072-
.block_on(async { read_payments(Arc::clone(&kv_store), Arc::clone(&logger)).await })
1073-
{
1074-
Ok(payments) => Arc::new(PaymentStore::new(
1075-
payments,
1076-
PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE.to_string(),
1077-
PAYMENT_INFO_PERSISTENCE_SECONDARY_NAMESPACE.to_string(),
1078-
Arc::clone(&kv_store),
1079-
Arc::clone(&logger),
1080-
)),
1081-
Err(e) => {
1082-
log_error!(logger, "Failed to read payment data from store: {}", e);
1083-
return Err(BuildError::ReadFailed);
1084-
},
1085-
};
1071+
let payment_store =
1072+
match runtime.block_on(async { read_payments(&*kv_store, Arc::clone(&logger)).await }) {
1073+
Ok(payments) => Arc::new(PaymentStore::new(
1074+
payments,
1075+
PAYMENT_INFO_PERSISTENCE_PRIMARY_NAMESPACE.to_string(),
1076+
PAYMENT_INFO_PERSISTENCE_SECONDARY_NAMESPACE.to_string(),
1077+
Arc::clone(&kv_store),
1078+
Arc::clone(&logger),
1079+
)),
1080+
Err(e) => {
1081+
log_error!(logger, "Failed to read payment data from store: {}", e);
1082+
return Err(BuildError::ReadFailed);
1083+
},
1084+
};
10861085

10871086
let (chain_source, chain_tip_opt) = match chain_data_source_config {
10881087
Some(ChainDataSourceConfig::Esplora { server_url, headers, sync_config }) => {
@@ -1298,7 +1297,7 @@ fn build_with_store_internal(
12981297

12991298
// Initialize the network graph, scorer, and router
13001299
let network_graph = match runtime
1301-
.block_on(async { read_network_graph(Arc::clone(&kv_store), Arc::clone(&logger)).await })
1300+
.block_on(async { read_network_graph(&*kv_store, Arc::clone(&logger)).await })
13021301
{
13031302
Ok(graph) => Arc::new(graph),
13041303
Err(e) => {
@@ -1312,7 +1311,7 @@ fn build_with_store_internal(
13121311
};
13131312

13141313
let local_scorer = match runtime.block_on(async {
1315-
read_scorer(Arc::clone(&kv_store), Arc::clone(&network_graph), Arc::clone(&logger)).await
1314+
read_scorer(&*kv_store, Arc::clone(&network_graph), Arc::clone(&logger)).await
13161315
}) {
13171316
Ok(scorer) => scorer,
13181317
Err(e) => {
@@ -1330,8 +1329,7 @@ fn build_with_store_internal(
13301329

13311330
// Restore external pathfinding scores from cache if possible.
13321331
match runtime.block_on(async {
1333-
read_external_pathfinding_scores_from_cache(Arc::clone(&kv_store), Arc::clone(&logger))
1334-
.await
1332+
read_external_pathfinding_scores_from_cache(&*kv_store, Arc::clone(&logger)).await
13351333
}) {
13361334
Ok(external_scores) => {
13371335
scorer.lock().unwrap().merge(external_scores, cur_time);
@@ -1490,15 +1488,11 @@ fn build_with_store_internal(
14901488
{
14911489
let mut locked_node_metrics = node_metrics.write().unwrap();
14921490
locked_node_metrics.latest_rgs_snapshot_timestamp = None;
1493-
write_node_metrics(
1494-
&*locked_node_metrics,
1495-
Arc::clone(&kv_store),
1496-
Arc::clone(&logger),
1497-
)
1498-
.map_err(|e| {
1499-
log_error!(logger, "Failed writing to store: {}", e);
1500-
BuildError::WriteFailed
1501-
})?;
1491+
write_node_metrics(&*locked_node_metrics, &*kv_store, Arc::clone(&logger))
1492+
.map_err(|e| {
1493+
log_error!(logger, "Failed writing to store: {}", e);
1494+
BuildError::WriteFailed
1495+
})?;
15021496
}
15031497
p2p_source
15041498
},

‎src/chain/bitcoind.rs‎

Lines changed: 11 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -205,14 +205,10 @@ impl BitcoindChainSource {
205205
unix_time_secs_opt;
206206
locked_node_metrics.latest_onchain_wallet_sync_timestamp =
207207
unix_time_secs_opt;
208-
write_node_metrics(
209-
&*locked_node_metrics,
210-
Arc::clone(&self.kv_store),
211-
Arc::clone(&self.logger),
212-
)
213-
.unwrap_or_else(|e| {
214-
log_error!(self.logger, "Failed to persist node metrics: {}", e);
215-
});
208+
write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)
209+
.unwrap_or_else(|e| {
210+
log_error!(self.logger, "Failed to persist node metrics: {}", e);
211+
});
216212
}
217213
break;
218214
},
@@ -420,11 +416,11 @@ impl BitcoindChainSource {
420416
*self.latest_chain_tip.write().unwrap() = Some(tip);
421417

422418
periodically_archive_fully_resolved_monitors(
423-
Arc::clone(&channel_manager),
424-
chain_monitor,
425-
Arc::clone(&self.kv_store),
426-
Arc::clone(&self.logger),
427-
Arc::clone(&self.node_metrics),
419+
&*channel_manager,
420+
&*chain_monitor,
421+
&*self.kv_store,
422+
&*self.logger,
423+
&*self.node_metrics,
428424
)?;
429425
},
430426
Ok(_) => {},
@@ -469,11 +465,7 @@ impl BitcoindChainSource {
469465
locked_node_metrics.latest_lightning_wallet_sync_timestamp = unix_time_secs_opt;
470466
locked_node_metrics.latest_onchain_wallet_sync_timestamp = unix_time_secs_opt;
471467

472-
write_node_metrics(
473-
&*locked_node_metrics,
474-
Arc::clone(&self.kv_store),
475-
Arc::clone(&self.logger),
476-
)?;
468+
write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
477469

478470
Ok(())
479471
}
@@ -586,11 +578,7 @@ impl BitcoindChainSource {
586578
{
587579
let mut locked_node_metrics = self.node_metrics.write().unwrap();
588580
locked_node_metrics.latest_fee_rate_cache_update_timestamp = unix_time_secs_opt;
589-
write_node_metrics(
590-
&*locked_node_metrics,
591-
Arc::clone(&self.kv_store),
592-
Arc::clone(&self.logger),
593-
)?;
581+
write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
594582
}
595583

596584
Ok(())

‎src/chain/electrum.rs‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -149,8 +149,8 @@ impl ElectrumChainSource {
149149
unix_time_secs_opt;
150150
write_node_metrics(
151151
&*locked_node_metrics,
152-
Arc::clone(&self.kv_store),
153-
Arc::clone(&self.logger),
152+
&*self.kv_store,
153+
&*self.logger,
154154
)?;
155155
}
156156
Ok(())
@@ -239,19 +239,15 @@ impl ElectrumChainSource {
239239
{
240240
let mut locked_node_metrics = self.node_metrics.write().unwrap();
241241
locked_node_metrics.latest_lightning_wallet_sync_timestamp = unix_time_secs_opt;
242-
write_node_metrics(
243-
&*locked_node_metrics,
244-
Arc::clone(&self.kv_store),
245-
Arc::clone(&self.logger),
246-
)?;
242+
write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
247243
}
248244

249245
periodically_archive_fully_resolved_monitors(
250-
Arc::clone(&channel_manager),
251-
Arc::clone(&chain_monitor),
252-
Arc::clone(&self.kv_store),
253-
Arc::clone(&self.logger),
254-
Arc::clone(&self.node_metrics),
246+
&*channel_manager,
247+
&*chain_monitor,
248+
&*self.kv_store,
249+
&*self.logger,
250+
&*self.node_metrics,
255251
)?;
256252
}
257253

@@ -284,11 +280,7 @@ impl ElectrumChainSource {
284280
{
285281
let mut locked_node_metrics = self.node_metrics.write().unwrap();
286282
locked_node_metrics.latest_fee_rate_cache_update_timestamp = unix_time_secs_opt;
287-
write_node_metrics(
288-
&*locked_node_metrics,
289-
Arc::clone(&self.kv_store),
290-
Arc::clone(&self.logger),
291-
)?;
283+
write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
292284
}
293285

294286
Ok(())

‎src/chain/esplora.rs‎

Lines changed: 9 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -128,8 +128,8 @@ impl EsploraChainSource {
128128
locked_node_metrics.latest_onchain_wallet_sync_timestamp = unix_time_secs_opt;
129129
write_node_metrics(
130130
&*locked_node_metrics,
131-
Arc::clone(&self.kv_store),
132-
Arc::clone(&self.logger)
131+
&*self.kv_store,
132+
&*self.logger
133133
)?;
134134
}
135135
Ok(())
@@ -259,19 +259,15 @@ impl EsploraChainSource {
259259
let mut locked_node_metrics = self.node_metrics.write().unwrap();
260260
locked_node_metrics.latest_lightning_wallet_sync_timestamp =
261261
unix_time_secs_opt;
262-
write_node_metrics(
263-
&*locked_node_metrics,
264-
Arc::clone(&self.kv_store),
265-
Arc::clone(&self.logger),
266-
)?;
262+
write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
267263
}
268264

269265
periodically_archive_fully_resolved_monitors(
270-
Arc::clone(&channel_manager),
271-
Arc::clone(&chain_monitor),
272-
Arc::clone(&self.kv_store),
273-
Arc::clone(&self.logger),
274-
Arc::clone(&self.node_metrics),
266+
&*channel_manager,
267+
&*chain_monitor,
268+
&*self.kv_store,
269+
&*self.logger,
270+
&*self.node_metrics,
275271
)?;
276272
Ok(())
277273
},
@@ -353,11 +349,7 @@ impl EsploraChainSource {
353349
{
354350
let mut locked_node_metrics = self.node_metrics.write().unwrap();
355351
locked_node_metrics.latest_fee_rate_cache_update_timestamp = unix_time_secs_opt;
356-
write_node_metrics(
357-
&*locked_node_metrics,
358-
Arc::clone(&self.kv_store),
359-
Arc::clone(&self.logger),
360-
)?;
352+
write_node_metrics(&*locked_node_metrics, &*self.kv_store, &*self.logger)?;
361353
}
362354

363355
Ok(())

‎src/chain/mod.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -488,8 +488,8 @@ impl Filter for ChainSource {
488488
}
489489

490490
fn periodically_archive_fully_resolved_monitors(
491-
channel_manager: Arc<ChannelManager>, chain_monitor: Arc<ChainMonitor>,
492-
kv_store: Arc<DynStore>, logger: Arc<Logger>, node_metrics: Arc<RwLock<NodeMetrics>>,
491+
channel_manager: &ChannelManager, chain_monitor: &ChainMonitor, kv_store: &DynStore,
492+
logger: &Logger, node_metrics: &RwLock<NodeMetrics>,
493493
) -> Result<(), Error> {
494494
let mut locked_node_metrics = node_metrics.write().unwrap();
495495
let cur_height = channel_manager.current_best_block().height;

‎src/io/utils.rs‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ pub(crate) fn read_or_generate_seed_file(
8888

8989
/// Read a previously persisted [`NetworkGraph`] from the store.
9090
pub(crate) async fn read_network_graph<L: Deref + Clone>(
91-
kv_store: Arc<DynStore>, logger: L,
91+
kv_store: &DynStore, logger: L,
9292
) -> Result<NetworkGraph<L>, std::io::Error>
9393
where
9494
L::Target: LdkLogger,
@@ -108,7 +108,7 @@ where
108108

109109
/// Read a previously persisted [`ProbabilisticScorer`] from the store.
110110
pub(crate) async fn read_scorer<G: Deref<Target = NetworkGraph<L>>, L: Deref + Clone>(
111-
kv_store: Arc<DynStore>, network_graph: G, logger: L,
111+
kv_store: &DynStore, network_graph: G, logger: L,
112112
) -> Result<ProbabilisticScorer<G, L>, std::io::Error>
113113
where
114114
L::Target: LdkLogger,
@@ -130,7 +130,7 @@ where
130130

131131
/// Read previously persisted external pathfinding scores from the cache.
132132
pub(crate) async fn read_external_pathfinding_scores_from_cache<L: Deref>(
133-
kv_store: Arc<DynStore>, logger: L,
133+
kv_store: &DynStore, logger: L,
134134
) -> Result<ChannelLiquidities, std::io::Error>
135135
where
136136
L::Target: LdkLogger,
@@ -150,7 +150,7 @@ where
150150

151151
/// Persist external pathfinding scores to the cache.
152152
pub(crate) async fn write_external_pathfinding_scores_to_cache<L: Deref>(
153-
kv_store: Arc<DynStore>, data: &ChannelLiquidities, logger: L,
153+
kv_store: &DynStore, data: &ChannelLiquidities, logger: L,
154154
) -> Result<(), Error>
155155
where
156156
L::Target: LdkLogger,
@@ -218,7 +218,7 @@ where
218218

219219
/// Read previously persisted payments information from the store.
220220
pub(crate) async fn read_payments<L: Deref>(
221-
kv_store: Arc<DynStore>, logger: L,
221+
kv_store: &DynStore, logger: L,
222222
) -> Result<Vec<PaymentDetails>, std::io::Error>
223223
where
224224
L::Target: LdkLogger,
@@ -323,7 +323,7 @@ pub(crate) async fn read_output_sweeper(
323323
}
324324

325325
pub(crate) async fn read_node_metrics<L: Deref>(
326-
kv_store: Arc<DynStore>, logger: L,
326+
kv_store: &DynStore, logger: L,
327327
) -> Result<NodeMetrics, std::io::Error>
328328
where
329329
L::Target: LdkLogger,
@@ -342,7 +342,7 @@ where
342342
}
343343

344344
pub(crate) fn write_node_metrics<L: Deref>(
345-
node_metrics: &NodeMetrics, kv_store: Arc<DynStore>, logger: L,
345+
node_metrics: &NodeMetrics, kv_store: &DynStore, logger: L,
346346
) -> Result<(), Error>
347347
where
348348
L::Target: LdkLogger,
@@ -469,7 +469,7 @@ macro_rules! impl_read_write_change_set_type {
469469
$key:expr
470470
) => {
471471
pub(crate) fn $read_name<L: Deref>(
472-
kv_store: Arc<DynStore>, logger: L,
472+
kv_store: &DynStore, logger: L,
473473
) -> Result<Option<$change_set_type>, std::io::Error>
474474
where
475475
L::Target: LdkLogger,
@@ -510,7 +510,7 @@ macro_rules! impl_read_write_change_set_type {
510510
}
511511

512512
pub(crate) fn $write_name<L: Deref>(
513-
value: &$change_set_type, kv_store: Arc<DynStore>, logger: L,
513+
value: &$change_set_type, kv_store: &DynStore, logger: L,
514514
) -> Result<(), std::io::Error>
515515
where
516516
L::Target: LdkLogger,
@@ -588,41 +588,35 @@ impl_read_write_change_set_type!(
588588

589589
// Reads the full BdkWalletChangeSet or returns default fields
590590
pub(crate) fn read_bdk_wallet_change_set(
591-
kv_store: Arc<DynStore>, logger: Arc<Logger>,
591+
kv_store: &DynStore, logger: &Logger,
592592
) -> Result<Option<BdkWalletChangeSet>, std::io::Error> {
593593
let mut change_set = BdkWalletChangeSet::default();
594594

595595
// We require a descriptor and return `None` to signal creation of a new wallet otherwise.
596-
if let Some(descriptor) =
597-
read_bdk_wallet_descriptor(Arc::clone(&kv_store), Arc::clone(&logger))?
598-
{
596+
if let Some(descriptor) = read_bdk_wallet_descriptor(kv_store, logger)? {
599597
change_set.descriptor = Some(descriptor);
600598
} else {
601599
return Ok(None);
602600
}
603601

604602
// We require a change_descriptor and return `None` to signal creation of a new wallet otherwise.
605-
if let Some(change_descriptor) =
606-
read_bdk_wallet_change_descriptor(Arc::clone(&kv_store), Arc::clone(&logger))?
607-
{
603+
if let Some(change_descriptor) = read_bdk_wallet_change_descriptor(kv_store, logger)? {
608604
change_set.change_descriptor = Some(change_descriptor);
609605
} else {
610606
return Ok(None);
611607
}
612608

613609
// We require a network and return `None` to signal creation of a new wallet otherwise.
614-
if let Some(network) = read_bdk_wallet_network(Arc::clone(&kv_store), Arc::clone(&logger))? {
610+
if let Some(network) = read_bdk_wallet_network(kv_store, logger)? {
615611
change_set.network = Some(network);
616612
} else {
617613
return Ok(None);
618614
}
619615

620-
read_bdk_wallet_local_chain(Arc::clone(&kv_store), Arc::clone(&logger))?
616+
read_bdk_wallet_local_chain(&*kv_store, logger)?
621617
.map(|local_chain| change_set.local_chain = local_chain);
622-
read_bdk_wallet_tx_graph(Arc::clone(&kv_store), Arc::clone(&logger))?
623-
.map(|tx_graph| change_set.tx_graph = tx_graph);
624-
read_bdk_wallet_indexer(Arc::clone(&kv_store), Arc::clone(&logger))?
625-
.map(|indexer| change_set.indexer = indexer);
618+
read_bdk_wallet_tx_graph(&*kv_store, logger)?.map(|tx_graph| change_set.tx_graph = tx_graph);
619+
read_bdk_wallet_indexer(&*kv_store, logger)?.map(|indexer| change_set.indexer = indexer);
626620
Ok(Some(change_set))
627621
}
628622

0 commit comments

Comments
 (0)