Repository navigation
fix(hotspots): avoid cartesian product in getImportStringFrom (stacked on #238) - #240
Merged
Merged
Conversation
Co-authored-by: felickz <1760475+felickz@users.noreply.github.com>
Co-authored-by: felickz <1760475+felickz@users.noreply.github.com>
…trings
getImportStringFrom compared the config file's dotted path against
Import.getImportString() with matches("%" + importString). That non-equality
string join gives the optimizer nothing to bind on, and with CodeQL 2.27.3
(github/codeql codeql-cli/v2.27.3) it planned a cartesian product of ~33k
import tuples with the 15M-row locations_default relation, filling the runner
disk.
Materialize the config's dotted-path suffixes in a pragma[nomagic] helper and
join on string equality instead.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…anches hotspots-ci.yml only triggers for PRs targeting main, so a PR stacked on chore/update-codeql-cli-* would get no hotspots signal. Drop this commit before merging the fix into the bump branch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
🔵 Needs a closer look
Completed target-CLI validation is still needed to confirm generation performance and the expected 294 configuration rows.
No open issues 🎉
What changed in this PR
This PR addresses excessive disk usage during hotspots query generation on CodeQL 2.27.3, stacked on the CLI bump in #238.
Changes:
- Materializes configuration-path suffixes and replaces wildcard matching with an equality join to avoid the problematic Cartesian product.
- Temporarily enables hotspots CI for CLI-update target branches; this workflow change is intended to be dropped before merging into the bump branch.
| File | Description |
|---|---|
ql/hotspots/utils/hotspots.qll |
Adds suffix helpers and uses equality to match configuration imports. |
.github/workflows/hotspots-ci.yml |
Temporarily enables validation for stacked CLI-update PRs. |
Review effort: Balanced
felickz
added a commit
that referenced
this pull request
Oct 27, 2026
…#240) (#242) * chore: bump pinned CodeQL CLI to v2.27.3 and release v0.8.3 * Add Hotspots query resource diagnostics Co-authored-by: felickz <1760475+felickz@users.noreply.github.com> * Cap Hotspots evaluator disk cache Co-authored-by: felickz <1760475+felickz@users.noreply.github.com> * fix(hotspots): avoid cartesian product when resolving config import strings getImportStringFrom compared the config file's dotted path against Import.getImportString() with matches("%" + importString). That non-equality string join gives the optimizer nothing to bind on, and with CodeQL 2.27.3 (github/codeql codeql-cli/v2.27.3) it planned a cartesian product of ~33k import tuples with the 15M-row locations_default relation, filling the runner disk. Materialize the config's dotted-path suffixes in a pragma[nomagic] helper and join on string equality instead. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * ci(hotspots): TEMPORARY run Hotspots CI on PRs stacked on CLI bump branches hotspots-ci.yml only triggers for PRs targeting main, so a PR stacked on chore/update-codeql-cli-* would get no hotspots signal. Drop this commit before merging the fix into the bump branch. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> * fix(ci): keep JVM warnings out of codeql test JSON reports `ci.yml` redirects each `codeql test run --format=json` process's stdout straight into `test_report_slice_N_of_4.json`. The JVM prints its own warnings to stdout too, so a stray line such as [0.001s][warning][perf,memops] Cannot use file /tmp/hsperfdata_runner/2405 because it is locked by another process (errno = 11) ahead of the JSON made `jq` fail in "Validate test results" even though every test passed. Four JVMs start at the same instant per language job, which makes the per-PID hsperfdata file collision possible. 1. Pass `-J=-XX:-UsePerfData` to `codeql test run` so the JVM never creates those files. 2. In "Validate test results", drop anything before the opening `[` of each report before running `jq`, emit a warning when something was dropped, and still fail if the remainder is not valid JSON. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #238 (CodeQL CLI 2.27.3 bump). Targets
chore/update-codeql-cli-2.27.3, notmain.Problem
In #238 the
Generate hotspots queriesjob fills the runner disk (/tmp/codeqldbgrows ~2.4 GB/min to ~83 GB). The evaluator log (from the diagnostics artifact) shows a single unfinished predicate, thegetImportStringFromjoin atql/hotspots/utils/hotspots.qll. Its plan containsJOIN WITH locations_default CARTESIAN PRODUCT: ~33k import tuples x 14.97M locations, ~5e11 tuples.The cause is a non-equality string join:
configPath.matches("%" + i.getImportString())gives the optimizer nothing to bind on. The 2.27.2 run did not hit this. Fourql/commits landed ingithub/codeqlbetweencodeql-cli/v2.27.2andv2.27.3, so a planner change is the likely trigger (still being investigated).Fix
pragma[nomagic]helper (getADottedPathSuffix).Import.getImportString()instead ofmatches("%" + ...).Result semantics are unchanged: a suffix match becomes an equality against every suffix. The one difference is that
_/%in an import string are no longer wildcards.Temporary CI commit
hotspots-ci.ymlonly triggers for PRs targetingmain, so this stacked PR would get no hotspots signal. The second commit widens the branch filter tochore/update-codeql-cli-*. Drop that commit before merging into the bump branch.Validation
codeql query compile(2.27.0 CLI against thegithub/codeqlv2.27.3qlpack). The wider checks need the CI run.Hotspots Build/Validateto finishGenerate hotspots queriesin about 4-5 min with 294 configuration rows (same as the 2.27.2 baseline).