feat(gc): record workspace manifest and target dir in global cache tracker by baby230211 · Pull Request #13846 · rust-lang/cargo

baby230211 · 2024-05-02T12:48:35Z

What does this PR try to resolve?

Fix #13136
This PR aims to track of the following data in GC for further garbage collection use.

root-manifest path
target directory path

There will be three new table for this features.

worksapce_manifest_index table - index and record the root manifest path

id	workspace_manifest	timestamp
?	/foo/Cargo.toml	?

target_dir_index table - index and record the target_dir path

id	target_dir	timestamp
?	/foo/target	?

workspace_src table - combine the previous two table

id	worksapce_id	target_dir_id	timestamp
?	?	?	?

How should we test and review this PR?

Run compile (cargo run build or else)
inspect your root gc file (~/.cargo/.global-cache)
see if the three table is record correctly.

rustbot · 2024-05-02T12:48:40Z

r? @ehuss

rustbot has assigned @ehuss.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

epage · 2024-05-02T13:18:37Z

+        basic_migration(
+            "CREATE TABLE workspace_manifest_index (
+                id INTEGER PRIMARY KEY AUTOINCREMENT,
+                name TEXT UNIQUE NOT NULL,
+                timestamp INTEGER NOT NULL
+            )",
+        ),
+        basic_migration(
+            "CREATE TABLE target_dir_index (
+                id INTEGER PRIMARY KEY AUTOINCREMENT,
+                name TEXT UNIQUE NOT NULL,
+                timestamp INTEGER NOT NULL
+            )",
+        ),
+        basic_migration(
+            "CREATE TABLE workspace_src (
+                workspace_id INTEGER NOT NULL,
+                target_dir_id INTEGER NOT NULL,
+                timestamp INTEGER NOT NULL,
+                PRIMARY KEY (workspace_id, target_dir_id),
+                FOREIGN KEY (workspace_id) REFERENCES workspace_manifest_index (id) ON DELETE CASCADE,
+                FOREIGN KEY (target_dir_id) REFERENCES target_dir_index (id) ON DELETE CASCADE
+            )",


Whats the motivation for

Tracking this as 3 separate tables

Having timestamps for workspace dir, target dir, and workspace dir + target dir

epage · 2024-05-02T13:20:02Z

+    /// New workspace manifest entries to insert.
+    workspace_db_timestamps: HashMap<WorkspaceManifestIndex, Timestamp>,
+    /// New target dir entries to insert.
+    target_dir_db_timestamps: HashMap<TargetDirIndex, Timestamp>,
+    /// New workspace src entries to insert.
+    workspace_src_timestamps: HashMap<WorkspaceSrc, Timestamp>,


What does db mean in this context?

I think for git

git_db is the .git

git_checkout is a work dir of a shared .git

epage · 2024-05-02T13:24:42Z

+    pub encoded_workspace_manifest_name: InternedString,
+    pub encoded_target_dir_name: InternedString,


When the others refer to "encoded" they are referring to how we translate package names to file names.

That doesn't quite make sense here and just naming these target_dir and workspace_manifest_path should work

i'll rename it with target_dir_path and workspace_manifest_path for these naming.
Thanks.

epage · 2024-05-02T13:25:24Z

+    /// Indicates the given [`WorkspaceManifest`] has been used right now.
+    ///
+    /// Also implicitly marks the workspace manifest used, too.
+    pub fn mark_workspace_src_used(&mut self, workspace_src: WorkspaceSrc) {


Huh, I wonder why we exposed these in the API, rather than taking parameters. Seems like it creates more boilerplate for the callers

epage · 2024-05-02T13:26:36Z

            HasDevUnits::No
        }
    };
+    let _ = &gctx.deferred_global_last_use()?.mark_workspace_src_used(


Why let _ = &?

If we remove let _ = &, rustc would be warning as follow

rustc: unused borrow that must be used (unused_must_use)

I removed it locally and didn't get any message...

gctx.deferred_global_last_use()? .mark_workspace_src_used(global_cache_tracker::WorkspaceSrc { workspace_manifest_name: InternedString::new(ws.root_manifest().to_str().unwrap()), target_dir_name: InternedString::new( ws.target_dir().as_path_unlocked().to_str().unwrap(), ), });

This is the error msg where runs on CI.
https://github.com/rust-lang/cargo/actions/runs/9055535414/job/24876799051#step:5:687

That has a leading & that can be removed

It works. Thanks, I've updated.

epage · 2024-05-02T13:27:35Z

        );
    }

+    // Flushes all of the `target_dir_db_timestamps` to the database,


Review note: I've not yet dug into all of the DB operations

bors · 2024-06-07T18:35:39Z

☔ The latest upstream changes (presumably #13979) made this pull request unmergeable. Please resolve the merge conflicts.

epage · 2024-06-20T01:19:24Z

As this is marked as a draft, @baby230211 what do you see as the next steps before marking this ready for review?

baby230211 · 2024-06-20T16:15:36Z

@epage , Sorry for pending this PR so long, the final step here is to record the manifest lock file in manifest table i think.

epage · 2024-06-21T15:30:58Z

You're ok. I just wanted to check if there was something waiting on me.

…acker

…recorded correctly

baby230211 · 2024-10-16T15:06:25Z

Hello @epage , i would like to re-work on this PR recently.
Due to recording the manifest lock file is not related to tracking the target/, i wonder if it's fine to not to track that lock file here.

epage · 2024-10-24T13:14:15Z

Lockfile tracking was mostly intended to prep for #13137. We'll likely need to database migrations anyways so probably fine to skip for now so long as we understand how we would support it in the future.

rustbot · 2025-12-15T16:12:57Z

☔ The latest upstream changes (possibly a7cdba1) made this pull request unmergeable. Please resolve the merge conflicts.

0xPoe · 2025-12-15T21:33:19Z

@rustbot author

rustbot · 2025-12-15T21:33:24Z

Reminder, once the PR becomes ready for a review, use @rustbot ready.

epage · 2026-01-28T15:39:17Z

As this has been waiting-on-author for a year, I'm going go ahead and close this. We are very much interested in this moving forward, whether by the original author or someone else.

rustbot assigned ehuss May 2, 2024

baby230211 marked this pull request as draft May 2, 2024 12:48

rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label May 2, 2024

baby230211 force-pushed the feat-13136 branch from f2d5a7d to 534397c Compare May 2, 2024 12:49

rustbot added the A-infrastructure Area: infrastructure around the cargo repo, ci, releases, etc. label May 2, 2024

baby230211 force-pushed the feat-13136 branch 3 times, most recently from 5f32f71 to 15e2863 Compare May 2, 2024 12:52

epage reviewed May 2, 2024

View reviewed changes

baby230211 force-pushed the feat-13136 branch 2 times, most recently from 9f069a9 to d55dda7 Compare May 15, 2024 12:39

baby230211 force-pushed the feat-13136 branch 5 times, most recently from 7d4fbbe to 481de08 Compare May 23, 2024 03:29

baby230211 force-pushed the feat-13136 branch from 481de08 to 37a7d5a Compare June 13, 2024 13:04

baby230211 added 2 commits August 21, 2024 10:46

feat(gc): record workspace manifest and target dir in global cache tr…

d542986

…acker

test(global_cache_tracker): verify workspace manifest and target dir …

7382a2d

…recorded correctly

baby230211 force-pushed the feat-13136 branch from 37a7d5a to 7382a2d Compare August 21, 2024 03:09

baby230211 marked this pull request as ready for review October 20, 2024 04:28

baby230211 changed the title ~~[WIP] feat(gc): record workspace manifest and target dir in global cache tracker~~ feat(gc): record workspace manifest and target dir in global cache tracker Oct 20, 2024

rustbot added S-waiting-on-author Status: The marked PR is awaiting some action (such as code changes) from the PR author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Dec 15, 2025

epage closed this Jan 28, 2026

epage mentioned this pull request Apr 22, 2026

gc: track and clean target directories #16929

Open

		pub encoded_workspace_manifest_name: InternedString,
		pub encoded_target_dir_name: InternedString,

Uh oh!

Conversation

baby230211 commented May 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What does this PR try to resolve?

How should we test and review this PR?

Uh oh!

rustbot commented May 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

baby230211 May 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bors commented Jun 7, 2024

Uh oh!

epage commented Jun 20, 2024

Uh oh!

baby230211 commented Jun 20, 2024

Uh oh!

epage commented Jun 21, 2024

Uh oh!

baby230211 commented Oct 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

epage commented Oct 24, 2024

Uh oh!

rustbot commented Dec 15, 2025

Uh oh!

0xPoe commented Dec 15, 2025

Uh oh!

rustbot commented Dec 15, 2025

Uh oh!

epage commented Jan 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

baby230211 commented May 2, 2024 •

edited

Loading

baby230211 May 22, 2024 •

edited

Loading

baby230211 commented Oct 16, 2024 •

edited

Loading