Skip to content

Go: stabilize build constraint evaluation - #22839

Merged
owen-mc merged 1 commit into
github:mainfrom
owen-mc:go/fix-dca-tuple-sum-noise
Nov 3, 2026
Merged

owen-mc merged 1 commit into
github:mainfrom
owen-mc:go/fix-dca-tuple-sum-noise

Conversation

@owen-mc

@owen-mc owen-mc commented Oct 16, 2026 •

Copy link
Copy Markdown
Contributor

Replace pragma[noinline] with pragma[nomagic] on ExtractedOrExternalFile.hasBuildConstraints() so that build-constrained files are computed bottom-up rather than using demand specialization.

Motivation

On the aws/aws-sdk-go database used for testing, there are only 24 files containing build constraints. However, the existing demand-specialized plan propagates demands from call targets and function bodies through location joins, producing approximately 15–19 million intermediate tuples before filtering by build-constraint comments.

With pragma[nomagic], the predicate instead starts from build-constraint comments and finds their files directly. Its evaluation processes 29 tuples and returns the 24 build-constrained files.

DCA issue

Nightly DCA experiments repeatedly reported tuple-sum changes for:

Files::ExtractedOrExternalFile.hasBuildConstraints/0#4828fb61#b

These occurred when analyzing identical source code with effectively unchanged CodeQL code. Repeated evaluations of the same query and database reproduced the variation: with fine-grained parallelism, the recorded intermediate tuple counts depend on worker scheduling and duplicate handling, despite unchanged query results.

This change removes the large, schedule-dependent join rather than changing DCA's metric or disabling parallel evaluation. The replacement plan consistently recorded 29 tuples in the repeated local evaluations.

@owen-mc
owen-mc requested a review from a team October 16, 2026 19:27
@owen-mc owen-mc added the no-change-note-required This PR does not need a change note label Oct 16, 2026
@github-actions github-actions Bot added the Go label Oct 16, 2026
@owen-mc

owen-mc commented Oct 17, 2026

Copy link
Copy Markdown
Contributor Author

I ran DCA. Tuple sums for that predicate seem much improved. There is a result for Log ERROR lines, per source (v1) that I need to look into.

@owen-mc
owen-mc marked this pull request as ready for review October 17, 2026 09:24
@owen-mc
owen-mc requested a review from a team as a code owner October 17, 2026 09:24
Copilot AI max review requested due to automatic review settings October 17, 2026 09:24

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟢 Approval recommended

The localized refactor preserves predicate semantics while establishing the intended evaluation boundary.

No open issues 🎉

What changed in this PR

Stabilizes Go build-constraint evaluation to reduce expensive location joins and recurring DCA noise without changing results.

Changes:

  • Adds a private bottom-up helper using pragma[nomagic].
  • Preserves demand specialization on the public hasBuildConstraints() predicate.
File Description
go/​ql/​lib/​semmle/​go/​Files.qll Separates build-constraint discovery from caller demand propagation.

Review effort: Max

@owen-mc
owen-mc requested a review from hvitved October 29, 2026 14:22
@owen-mc

owen-mc commented Oct 29, 2026

Copy link
Copy Markdown
Contributor Author

I couldn't find the Log ERROR lines, per source (v1) results in the actions logs.

@owen-mc
owen-mc requested a review from aschackmull November 2, 2026 18:32
Comment thread go/ql/lib/semmle/go/Files.qll Resolved
@owen-mc
owen-mc force-pushed the go/fix-dca-tuple-sum-noise branch from d789aa3 to c61ea69 Compare November 3, 2026 10:55
hvitved
hvitved previously approved these changes Nov 3, 2026
@owen-mc

owen-mc commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

I previously tried having both pragma[noinline] and pragma[nomagic] (which I suspect now is probably redundant), and found it slower, hence why I came up with what was in this PR. But maybe I made a mistake because I just tried your suggestion and it wasn't slower. I've force-pushed to just use pragma[nomagic]. I've started another DCA run.

@hvitved

hvitved commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

I previously tried having both pragma[noinline] and pragma[nomagic] (which I suspect now is probably redundant), and found it slower, hence why I came up with what was in this PR. But maybe I made a mistake because I just tried your suggestion and it wasn't slower. I've force-pushed to just use pragma[nomagic]. I've started another DCA run.

Yes, nomagic implies noinline.

@owen-mc

owen-mc commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor Author

DCA is fine: no significant change to the overall analysis time, but a big decrease in tuple sums for one predicate.

@owen-mc
owen-mc merged commit 869263c into github:main Nov 3, 2026
25 checks passed
@owen-mc
owen-mc deleted the go/fix-dca-tuple-sum-noise branch November 3, 2026 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Go no-change-note-required This PR does not need a change note

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants