Skip to content

Commit df18c53

Browse files
committed
fix(cli): Trace core cargo operations
This is preparation for #13339 and covers hot spots I found as well as areas currently covered by `profile::start(...)`. This is split out to avoid conflicts while working through the remaining issues for #13339. Maybe it will also serve to help debugging...
1 parent 9e6288e commit df18c53

File tree

19 files changed

+42
-0
lines changed

19 files changed

+42
-0
lines changed

src/bin/cargo/cli.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ use crate::util::is_rustup;
1616
use cargo::core::shell::ColorChoice;
1717
use cargo::util::style;
1818

19+
#[tracing::instrument(skip_all)]
1920
pub fn main(gctx: &mut GlobalContext) -> CliResult {
2021
// CAUTION: Be careful with using `config` until it is configured below.
2122
// In general, try to avoid loading config values unless necessary (like
@@ -272,6 +273,7 @@ fn add_ssl(version_string: &mut String) {
272273
/// [`GlobalArgs`] need to be extracted before expanding aliases because the
273274
/// clap code for extracting a subcommand discards global options
274275
/// (appearing before the subcommand).
276+
#[tracing::instrument(skip_all)]
275277
fn expand_aliases(
276278
gctx: &mut GlobalContext,
277279
args: ArgMatches,
@@ -377,6 +379,7 @@ For more information, see issue #12207 <https://github.com/rust-lang/cargo/issue
377379
Ok((args, GlobalArgs::default()))
378380
}
379381

382+
#[tracing::instrument(skip_all)]
380383
fn configure_gctx(
381384
gctx: &mut GlobalContext,
382385
args: &ArgMatches,
@@ -459,6 +462,7 @@ impl Exec {
459462
}
460463
}
461464

465+
#[tracing::instrument(skip_all)]
462466
fn exec(self, gctx: &mut GlobalContext, subcommand_args: &ArgMatches) -> CliResult {
463467
match self {
464468
Self::Builtin(exec) => exec(gctx, subcommand_args),
@@ -530,6 +534,7 @@ impl GlobalArgs {
530534
}
531535
}
532536

537+
#[tracing::instrument(skip_all)]
533538
pub fn cli(gctx: &GlobalContext) -> Command {
534539
// Don't let config errors get in the way of parsing arguments
535540
let term = gctx.get::<TermConfig>("term").unwrap_or_default();

src/bin/cargo/main.rs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -287,6 +287,7 @@ fn search_directories(gctx: &GlobalContext) -> Vec<PathBuf> {
287287
}
288288

289289
/// Initialize libgit2.
290+
#[tracing::instrument(skip_all)]
290291
fn init_git(gctx: &GlobalContext) {
291292
// Disabling the owner validation in git can, in theory, lead to code execution
292293
// vulnerabilities. However, libgit2 does not launch executables, which is the foundation of
@@ -318,6 +319,7 @@ fn init_git(gctx: &GlobalContext) {
318319
/// If the user has a non-default network configuration, then libgit2 will be
319320
/// configured to use libcurl instead of the built-in networking support so
320321
/// that those configuration settings can be used.
322+
#[tracing::instrument(skip_all)]
321323
fn init_git_transports(gctx: &GlobalContext) {
322324
match needs_custom_http_transport(gctx) {
323325
Ok(true) => {}

src/cargo/core/compiler/build_runner/mod.rs

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
133133
/// See [`ops::cargo_compile`] for a higher-level view of the compile process.
134134
///
135135
/// [`ops::cargo_compile`]: ../../../ops/cargo_compile/index.html
136+
#[tracing::instrument(skip_all)]
136137
pub fn compile(mut self, exec: &Arc<dyn Executor>) -> CargoResult<Compilation<'gctx>> {
137138
// A shared lock is held during the duration of the build since rustc
138139
// needs to read from the `src` cache, and we don't want other
@@ -324,6 +325,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
324325
.map(|output| output.bin_dst().clone()))
325326
}
326327

328+
#[tracing::instrument(skip_all)]
327329
pub fn prepare_units(&mut self) -> CargoResult<()> {
328330
let dest = self.bcx.profiles.get_dir_name();
329331
let host_layout = Layout::new(self.bcx.ws, None, &dest)?;
@@ -349,6 +351,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
349351

350352
/// Prepare this context, ensuring that all filesystem directories are in
351353
/// place.
354+
#[tracing::instrument(skip_all)]
352355
pub fn prepare(&mut self) -> CargoResult<()> {
353356
let _p = profile::start("preparing layout");
354357

@@ -451,6 +454,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
451454

452455
/// Check if any output file name collision happens.
453456
/// See <https://github.com/rust-lang/cargo/issues/6313> for more.
457+
#[tracing::instrument(skip_all)]
454458
fn check_collisions(&self) -> CargoResult<()> {
455459
let mut output_collisions = HashMap::new();
456460
let describe_collision = |unit: &Unit, other_unit: &Unit, path: &PathBuf| -> String {
@@ -633,6 +637,7 @@ impl<'a, 'gctx> BuildRunner<'a, 'gctx> {
633637
/// If the current crate has reverse-dependencies, such a Check unit should exist, and so
634638
/// we use that crate's metadata. If not, we use the crate's Doc unit so at least examples
635639
/// scraped from the current crate can be used when documenting the current crate.
640+
#[tracing::instrument(skip_all)]
636641
pub fn compute_metadata_for_doc_units(&mut self) {
637642
for unit in self.bcx.unit_graph.keys() {
638643
if !unit.mode.is_doc() && !unit.mode.is_doc_scrape() {

src/cargo/core/compiler/custom_build.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ impl LinkArgTarget {
194194
}
195195

196196
/// Prepares a `Work` that executes the target as a custom build script.
197+
#[tracing::instrument(skip_all)]
197198
pub fn prepare(build_runner: &mut BuildRunner<'_, '_>, unit: &Unit) -> CargoResult<Job> {
198199
let _p = profile::start(format!(
199200
"build script prepare: {}/{}",

src/cargo/core/compiler/fingerprint/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,7 @@ pub use dirty_reason::DirtyReason;
399399
/// transitively propagate throughout the dependency graph, it only forces this
400400
/// one unit which is very unlikely to be what you want unless you're
401401
/// exclusively talking about top-level units.
402+
#[tracing::instrument(skip(build_runner, unit))]
402403
pub fn prepare_target(
403404
build_runner: &mut BuildRunner<'_, '_>,
404405
unit: &Unit,

src/cargo/core/compiler/job_queue/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,6 +467,7 @@ impl<'gctx> JobQueue<'gctx> {
467467
/// This function will spawn off `config.jobs()` workers to build all of the
468468
/// necessary dependencies, in order. Freshness is propagated as far as
469469
/// possible along each dependency chain.
470+
#[tracing::instrument(skip_all)]
470471
pub fn execute(
471472
mut self,
472473
build_runner: &mut BuildRunner<'_, '_>,

src/cargo/core/compiler/mod.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,7 @@ impl Executor for DefaultExecutor {
158158
/// Note that **no actual work is executed as part of this**, that's all done
159159
/// next as part of [`JobQueue::execute`] function which will run everything
160160
/// in order with proper parallelism.
161+
#[tracing::instrument(skip(build_runner, jobs, plan, exec))]
161162
fn compile<'gctx>(
162163
build_runner: &mut BuildRunner<'_, 'gctx>,
163164
jobs: &mut JobQueue<'gctx>,

src/cargo/core/compiler/unit_dependencies.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -81,6 +81,7 @@ impl IsArtifact {
8181
/// Then entry point for building a dependency graph of compilation units.
8282
///
8383
/// You can find some information for arguments from doc of [`State`].
84+
#[tracing::instrument(skip_all)]
8485
pub fn build_unit_dependencies<'a, 'gctx>(
8586
ws: &'a Workspace<'gctx>,
8687
package_set: &'a PackageSet<'gctx>,

src/cargo/core/global_cache_tracker.rs

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -546,6 +546,7 @@ impl GlobalCacheTracker {
546546
.with_context(|| "failed to clean entries from the global cache")
547547
}
548548

549+
#[tracing::instrument(skip_all)]
549550
fn clean_inner(
550551
&mut self,
551552
clean_ctx: &mut CleanContext<'_>,
@@ -696,6 +697,7 @@ impl GlobalCacheTracker {
696697
///
697698
/// These orphaned files will be added to `delete_paths` so that the
698699
/// caller can delete them.
700+
#[tracing::instrument(skip_all)]
699701
fn sync_db_with_files(
700702
conn: &Connection,
701703
now: Timestamp,
@@ -795,6 +797,7 @@ impl GlobalCacheTracker {
795797
}
796798

797799
/// For parent tables, add any entries that are on disk but aren't tracked in the db.
800+
#[tracing::instrument(skip_all)]
798801
fn update_parent_for_missing_from_db(
799802
conn: &Connection,
800803
now: Timestamp,
@@ -822,6 +825,7 @@ impl GlobalCacheTracker {
822825
///
823826
/// This could happen for example if the user manually deleted the file or
824827
/// any such scenario where the filesystem and db are out of sync.
828+
#[tracing::instrument(skip_all)]
825829
fn update_db_for_removed(
826830
conn: &Connection,
827831
parent_table_name: &str,
@@ -851,6 +855,7 @@ impl GlobalCacheTracker {
851855
}
852856

853857
/// Removes database entries for any files that are not on disk for the parent tables.
858+
#[tracing::instrument(skip_all)]
854859
fn update_db_parent_for_removed_from_disk(
855860
conn: &Connection,
856861
parent_table_name: &str,
@@ -888,6 +893,7 @@ impl GlobalCacheTracker {
888893
/// Updates the database to add any `.crate` files that are currently
889894
/// not tracked (such as when they are downloaded by an older version of
890895
/// cargo).
896+
#[tracing::instrument(skip_all)]
891897
fn populate_untracked_crate(
892898
conn: &Connection,
893899
now: Timestamp,
@@ -922,6 +928,7 @@ impl GlobalCacheTracker {
922928

923929
/// Updates the database to add any files that are currently not tracked
924930
/// (such as when they are downloaded by an older version of cargo).
931+
#[tracing::instrument(skip_all)]
925932
fn populate_untracked(
926933
conn: &Connection,
927934
now: Timestamp,
@@ -987,6 +994,7 @@ impl GlobalCacheTracker {
987994
/// size.
988995
///
989996
/// `update_db_for_removed` should be called before this is called.
997+
#[tracing::instrument(skip_all)]
990998
fn update_null_sizes(
991999
conn: &Connection,
9921000
gctx: &GlobalContext,
@@ -1560,6 +1568,7 @@ impl DeferredGlobalLastUse {
15601568
/// Saves all of the deferred information to the database.
15611569
///
15621570
/// This will also clear the state of `self`.
1571+
#[tracing::instrument(skip_all)]
15631572
pub fn save(&mut self, tracker: &mut GlobalCacheTracker) -> CargoResult<()> {
15641573
let _p = crate::util::profile::start("saving last-use data");
15651574
trace!(target: "gc", "saving last-use data");

src/cargo/core/package.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -497,6 +497,7 @@ impl<'gctx> PackageSet<'gctx> {
497497
}
498498

499499
/// Downloads any packages accessible from the give root ids.
500+
#[tracing::instrument(skip_all)]
500501
pub fn download_accessible(
501502
&self,
502503
resolve: &Resolve,

0 commit comments

Comments
 (0)