Skip to content

Commit 4451974

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 070810b commit 4451974

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
@@ -1052,7 +1052,7 @@ fn build_with_store_internal(
10521052

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

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

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

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

13131312
let local_scorer = match runtime.block_on(async {
1314-
read_scorer(Arc::clone(&kv_store), Arc::clone(&network_graph), Arc::clone(&logger)).await
1313+
read_scorer(&*kv_store, Arc::clone(&network_graph), Arc::clone(&logger)).await
13151314
}) {
13161315
Ok(scorer) => scorer,
13171316
Err(e) => {
@@ -1329,8 +1328,7 @@ fn build_with_store_internal(
13291328

13301329
// Restore external pathfinding scores from cache if possible.
13311330
match runtime.block_on(async {
1332-
read_external_pathfinding_scores_from_cache(Arc::clone(&kv_store), Arc::clone(&logger))
1333-
.await
1331+
read_external_pathfinding_scores_from_cache(&*kv_store, Arc::clone(&logger)).await
13341332
}) {
13351333
Ok(external_scores) => {
13361334
scorer.lock().unwrap().merge(external_scores, cur_time);
@@ -1487,15 +1485,11 @@ fn build_with_store_internal(
14871485
{
14881486
let mut locked_node_metrics = node_metrics.write().unwrap();
14891487
locked_node_metrics.latest_rgs_snapshot_timestamp = None;
1490-
write_node_metrics(
1491-
&*locked_node_metrics,
1492-
Arc::clone(&kv_store),
1493-
Arc::clone(&logger),
1494-
)
1495-
.map_err(|e| {
1496-
log_error!(logger, "Failed writing to store: {}", e);
1497-
BuildError::WriteFailed
1498-
})?;
1488+
write_node_metrics(&*locked_node_metrics, &*kv_store, Arc::clone(&logger))
1489+
.map_err(|e| {
1490+
log_error!(logger, "Failed writing to store: {}", e);
1491+
BuildError::WriteFailed
1492+
})?;
14991493
}
15001494
p2p_source
15011495
},

‎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,
@@ -313,7 +313,7 @@ pub(crate) async fn read_output_sweeper(
313313
}
314314

315315
pub(crate) async fn read_node_metrics<L: Deref>(
316-
kv_store: Arc<DynStore>, logger: L,
316+
kv_store: &DynStore, logger: L,
317317
) -> Result<NodeMetrics, std::io::Error>
318318
where
319319
L::Target: LdkLogger,
@@ -332,7 +332,7 @@ where
332332
}
333333

334334
pub(crate) fn write_node_metrics<L: Deref>(
335-
node_metrics: &NodeMetrics, kv_store: Arc<DynStore>, logger: L,
335+
node_metrics: &NodeMetrics, kv_store: &DynStore, logger: L,
336336
) -> Result<(), Error>
337337
where
338338
L::Target: LdkLogger,
@@ -459,7 +459,7 @@ macro_rules! impl_read_write_change_set_type {
459459
$key:expr
460460
) => {
461461
pub(crate) fn $read_name<L: Deref>(
462-
kv_store: Arc<DynStore>, logger: L,
462+
kv_store: &DynStore, logger: L,
463463
) -> Result<Option<$change_set_type>, std::io::Error>
464464
where
465465
L::Target: LdkLogger,
@@ -500,7 +500,7 @@ macro_rules! impl_read_write_change_set_type {
500500
}
501501

502502
pub(crate) fn $write_name<L: Deref>(
503-
value: &$change_set_type, kv_store: Arc<DynStore>, logger: L,
503+
value: &$change_set_type, kv_store: &DynStore, logger: L,
504504
) -> Result<(), std::io::Error>
505505
where
506506
L::Target: LdkLogger,
@@ -578,41 +578,35 @@ impl_read_write_change_set_type!(
578578

579579
// Reads the full BdkWalletChangeSet or returns default fields
580580
pub(crate) fn read_bdk_wallet_change_set(
581-
kv_store: Arc<DynStore>, logger: Arc<Logger>,
581+
kv_store: &DynStore, logger: &Logger,
582582
) -> Result<Option<BdkWalletChangeSet>, std::io::Error> {
583583
let mut change_set = BdkWalletChangeSet::default();
584584

585585
// We require a descriptor and return `None` to signal creation of a new wallet otherwise.
586-
if let Some(descriptor) =
587-
read_bdk_wallet_descriptor(Arc::clone(&kv_store), Arc::clone(&logger))?
588-
{
586+
if let Some(descriptor) = read_bdk_wallet_descriptor(kv_store, logger)? {
589587
change_set.descriptor = Some(descriptor);
590588
} else {
591589
return Ok(None);
592590
}
593591

594592
// We require a change_descriptor and return `None` to signal creation of a new wallet otherwise.
595-
if let Some(change_descriptor) =
596-
read_bdk_wallet_change_descriptor(Arc::clone(&kv_store), Arc::clone(&logger))?
597-
{
593+
if let Some(change_descriptor) = read_bdk_wallet_change_descriptor(kv_store, logger)? {
598594
change_set.change_descriptor = Some(change_descriptor);
599595
} else {
600596
return Ok(None);
601597
}
602598

603599
// We require a network and return `None` to signal creation of a new wallet otherwise.
604-
if let Some(network) = read_bdk_wallet_network(Arc::clone(&kv_store), Arc::clone(&logger))? {
600+
if let Some(network) = read_bdk_wallet_network(kv_store, logger)? {
605601
change_set.network = Some(network);
606602
} else {
607603
return Ok(None);
608604
}
609605

610-
read_bdk_wallet_local_chain(Arc::clone(&kv_store), Arc::clone(&logger))?
606+
read_bdk_wallet_local_chain(&*kv_store, logger)?
611607
.map(|local_chain| change_set.local_chain = local_chain);
612-
read_bdk_wallet_tx_graph(Arc::clone(&kv_store), Arc::clone(&logger))?
613-
.map(|tx_graph| change_set.tx_graph = tx_graph);
614-
read_bdk_wallet_indexer(Arc::clone(&kv_store), Arc::clone(&logger))?
615-
.map(|indexer| change_set.indexer = indexer);
608+
read_bdk_wallet_tx_graph(&*kv_store, logger)?.map(|tx_graph| change_set.tx_graph = tx_graph);
609+
read_bdk_wallet_indexer(&*kv_store, logger)?.map(|indexer| change_set.indexer = indexer);
616610
Ok(Some(change_set))
617611
}
618612

0 commit comments

Comments
 (0)