Skip to content

[clang][deps] Avoid CompilerInvocation copies - #13201

Merged
jansvoboda11 merged 3 commits into
nextfrom
jan_svoboda/invocation-copies
Jun 23, 2026
Merged

[clang][deps] Avoid CompilerInvocation copies#13201
jansvoboda11 merged 3 commits into
nextfrom
jan_svoboda/invocation-copies

Conversation

@jansvoboda11

Copy link
Copy Markdown

When constructing the dependency graph for compilation caching, the dependency scanner needs to do some extra operations on the compiler invocations. Historically, these have not utilized the copy-on-write variant well. This patch takes care to minimize CompilerInvocation copies, which improves incremental scans with populated up-to-date scanning module cache by 16-18%. Together with llvm#203350 which operates in the same space, wall-times are improved by 1.54x and instruction counts by 1.66x.

@jansvoboda11

Copy link
Copy Markdown
Author

Ping.

Comment thread clang/include/clang/DependencyScanning/ScanAndUpdateArgs.h
Comment thread clang/include/clang/Frontend/CompilerInvocation.h Outdated
@jansvoboda11

Copy link
Copy Markdown
Author

By propagating cow upwards, I realized most of my withTempCow() lambdas assigned the modified cow into this. I changed the interface a little bit in 7cbd7f7 so that this happens automatically in the non-const overload. LMK if you find this cleaner.

@jansvoboda11
jansvoboda11 requested a review from benlangmuir June 18, 2026 22:22
@jansvoboda11

Copy link
Copy Markdown
Author

Ping.

@benlangmuir benlangmuir left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mostly LGTM; couple of questions

Comment thread clang/include/clang/Frontend/CompilerInvocation.h Outdated
llvm::function_ref<bool(std::string &)> Callback) {
// Ensure exclusive ownership of every option group, so that visitPathsImpl()
// doesn't affect any other invocations.
// FIXME: Do this only if \c Callback does decide to modify any strings in an

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this implementable without a CowString?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's my next patch. I'll change the signature of Callback to say whether it wants to change the string and call ensureOwned on the owning options object before performing the modification.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants