Skip to content

Commit fe5c700

Browse files
camc314camc314
authored
refactor: remove redundant Arc around tracing spans (#9778)
`tracing::Span` is already a cheap, cloneable handle. Cloning it does not clone the full span data or create a new tracing span; it just creates another handle pointing at the same underlying span state. That means Arc<tracing::Span> is redundant here: ``` Arc::clone(&span) ``` and ``` span.clone() ``` both represent shared ownership of the same span handle, but `Arc` adds an extra allocation/refcount layer around a type that already has its own cheap handle semantics.
1 parent 199f7af commit fe5c700

6 files changed

Lines changed: 23 additions & 24 deletions

File tree

‎crates/rolldown/src/bundle/bundle.rs‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -36,11 +36,11 @@ pub struct Bundle<Fs: FileSystem + Clone + 'static = OsFileSystem> {
3636
pub(crate) plugin_driver: SharedPluginDriver,
3737
pub(crate) warnings: Vec<BuildDiagnostic>,
3838
pub(crate) cache: ScanStageCache,
39-
pub(crate) bundle_span: Arc<tracing::Span>,
39+
pub(crate) bundle_span: tracing::Span,
4040
}
4141

4242
impl<Fs: FileSystem + Clone + 'static> Bundle<Fs> {
43-
#[tracing::instrument(level = "debug", skip_all, parent = &*self.bundle_span)]
43+
#[tracing::instrument(level = "debug", skip_all, parent = &self.bundle_span)]
4444
/// This method intentionally get the ownership of `self` to show that the method cannot be called multiple times.
4545
pub async fn write(mut self) -> BuildResult<BundleOutput> {
4646
let start = self.plugin_driver.start_timing();
@@ -58,7 +58,7 @@ impl<Fs: FileSystem + Clone + 'static> Bundle<Fs> {
5858
self.append_plugin_timings_warning(result)
5959
}
6060

61-
#[tracing::instrument(level = "debug", skip_all, parent = &*self.bundle_span)]
61+
#[tracing::instrument(level = "debug", skip_all, parent = &self.bundle_span)]
6262
/// This method intentionally get the ownership of `self` to show that the method cannot be called multiple times.
6363
pub async fn generate(mut self) -> BuildResult<BundleOutput> {
6464
let start = self.plugin_driver.start_timing();
@@ -79,15 +79,15 @@ impl<Fs: FileSystem + Clone + 'static> Bundle<Fs> {
7979
self.append_plugin_timings_warning(result)
8080
}
8181

82-
#[tracing::instrument(level = "debug", skip_all, parent = &*self.bundle_span)]
82+
#[tracing::instrument(level = "debug", skip_all, parent = &self.bundle_span)]
8383
/// This method intentionally get the ownership of `self` to show that the method cannot be called multiple times.
8484
pub async fn scan(mut self) -> BuildResult<()> {
8585
self.scan_modules(ScanMode::Full).await?;
8686

8787
Ok(())
8888
}
8989

90-
#[tracing::instrument(level = "debug", skip_all, parent = &*self.bundle_span)]
90+
#[tracing::instrument(level = "debug", skip_all, parent = &self.bundle_span)]
9191
pub async fn scan_modules(
9292
&mut self,
9393
scan_mode: ScanMode<ArcStr>,
@@ -151,7 +151,7 @@ impl<Fs: FileSystem + Clone + 'static> Bundle<Fs> {
151151
}
152152
}
153153

154-
#[tracing::instrument(level = "debug", skip_all, parent = &*self.bundle_span)]
154+
#[tracing::instrument(level = "debug", skip_all, parent = &self.bundle_span)]
155155
pub async fn bundle_write(
156156
&mut self,
157157
scan_stage_output: NormalizedScanStageOutput,
@@ -220,7 +220,7 @@ impl<Fs: FileSystem + Clone + 'static> Bundle<Fs> {
220220
Ok(output)
221221
}
222222

223-
#[tracing::instrument(level = "debug", skip_all, parent = &*self.bundle_span)]
223+
#[tracing::instrument(level = "debug", skip_all, parent = &self.bundle_span)]
224224
pub async fn bundle_generate(
225225
&mut self,
226226
scan_stage_output: NormalizedScanStageOutput,

‎crates/rolldown/src/bundle/bundle_factory.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -93,17 +93,17 @@ impl BundleFactory {
9393
})
9494
}
9595

96-
fn generate_unique_bundle_span(&mut self) -> Arc<tracing::Span> {
96+
fn generate_unique_bundle_span(&mut self) -> tracing::Span {
9797
let bundle_id = rolldown_devtools::generate_build_id(self.bundle_id_seed);
9898
self.bundle_id_seed += 1;
99-
Arc::new(tracing::info_span!(
99+
tracing::info_span!(
100100
parent: &self.session.span,
101101
"build",
102102
CONTEXT_build_id = bundle_id.as_ref(),
103103
// - This behaves like default value for `${hook_resolve_id_trigger}`.
104104
// - For case like injecting `manual`, we will override this field by adding a child span to shadow this one.
105105
CONTEXT_hook_resolve_id_trigger = "automatic"
106-
))
106+
)
107107
}
108108

109109
pub fn create_bundle(

‎crates/rolldown_plugin/src/plugin_context/native_plugin_context.rs‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -42,11 +42,11 @@ pub struct NativePluginContextImpl {
4242
pub(crate) module_infos: SharedModuleInfoDashMap,
4343
pub(crate) tx: Arc<Mutex<Option<tokio::sync::mpsc::UnboundedSender<ModuleLoaderMsg>>>>,
4444
pub(crate) session: rolldown_devtools::Session,
45-
pub(crate) bundle_span: Arc<tracing::Span>,
45+
pub(crate) bundle_span: tracing::Span,
4646
// `resolve_id` hook not only will be triggered by the rolldown's resolve process, but also could be triggered
4747
// by manual calls of `PluginContext.resolve()`. We use a dedicated span here to distinguish whether the call is
4848
// - automatic (by rolldown) or manual (by `PluginContext.resolve()`)
49-
pub(crate) manual_resolve_span: Arc<tracing::Span>,
49+
pub(crate) manual_resolve_span: tracing::Span,
5050
}
5151

5252
impl NativePluginContextImpl {
@@ -129,7 +129,7 @@ impl NativePluginContextImpl {
129129
)
130130
.await
131131
}
132-
.instrument(self.manual_resolve_span.as_ref().clone())
132+
.instrument(self.manual_resolve_span.clone())
133133
.await
134134
}
135135

‎crates/rolldown_plugin/src/plugin_context/plugin_context.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -62,8 +62,8 @@ impl PluginContext {
6262
module_infos: Arc::clone(&ctx.module_infos),
6363
tx: Arc::clone(&ctx.tx),
6464
session: ctx.session.clone(),
65-
bundle_span: Arc::clone(&ctx.bundle_span),
66-
manual_resolve_span: Arc::clone(&ctx.manual_resolve_span),
65+
bundle_span: ctx.bundle_span.clone(),
66+
manual_resolve_span: ctx.manual_resolve_span.clone(),
6767
})),
6868
}
6969
}

‎crates/rolldown_plugin/src/plugin_driver/plugin_driver_factory.rs‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ impl PluginDriverFactory {
3434
file_emitter: &SharedFileEmitter,
3535
options: &SharedNormalizedBundlerOptions,
3636
session: &rolldown_devtools::Session,
37-
initial_bundle_span: &Arc<tracing::Span>,
37+
initial_bundle_span: &tracing::Span,
3838
module_infos: SharedModuleInfoDashMap,
3939
transform_dependencies: Arc<DashMap<ModuleIdx, Arc<FxDashSet<ArcStr>>>>,
4040
) -> Arc<crate::plugin_driver::PluginDriver> {
@@ -43,15 +43,14 @@ impl PluginDriverFactory {
4343
let tx = Arc::new(std::sync::Mutex::new(None));
4444
let mut plugin_usage_vec = IndexVec::new();
4545

46-
// Clone the Arc to share across contexts
47-
let bundle_span_arc = Arc::clone(initial_bundle_span);
46+
let bundle_span = initial_bundle_span.clone();
4847

4948
// Create derived span for manual resolve calls
50-
let manual_resolve_span_arc = Arc::new(tracing::debug_span!(
51-
parent: bundle_span_arc.as_ref(),
49+
let manual_resolve_span = tracing::debug_span!(
50+
parent: &bundle_span,
5251
"plugin_context_resolve",
5352
CONTEXT_hook_resolve_id_trigger = "manual"
54-
));
53+
);
5554

5655
// Create timing collector only if checks.pluginTimings is enabled
5756
let hook_timing_collector = if options.checks.contains(EventKindSwitcher::PluginTimings) {
@@ -89,8 +88,8 @@ impl PluginDriverFactory {
8988
watch_files: Arc::clone(&watch_files),
9089
tx: Arc::clone(&tx),
9190
session: session.clone(),
92-
bundle_span: Arc::clone(&bundle_span_arc),
93-
manual_resolve_span: Arc::clone(&manual_resolve_span_arc),
91+
bundle_span: bundle_span.clone(),
92+
manual_resolve_span: manual_resolve_span.clone(),
9493
})));
9594
});
9695

‎meta/design/rust-bundler.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,7 @@ pub struct Bundle {
6363
plugin_driver: SharedPluginDriver,
6464
warnings: Vec<BuildDiagnostic>,
6565
cache: ScanStageCache,
66-
bundle_span: Arc<tracing::Span>,
66+
bundle_span: tracing::Span,
6767
}
6868
```
6969

0 commit comments

Comments
 (0)