-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Java: Make some query libraries local. #20598
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR optimizes performance in Java query libraries by adding the overlay[local?]
annotation to expensive predicates. The purpose is to make certain predicates local to improve query execution performance, particularly for predicates that contain slow operations like regular expressions.
Key changes:
- Added
overlay[local?]
annotations to three expensive predicate classes - Targets predicates with known performance issues due to regex operations and other expensive computations
- Maintains existing functionality while improving execution efficiency
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
File | Description |
---|---|
java/ql/lib/semmle/code/java/security/UrlForwardQuery.qll |
Added local overlay annotation to BarrierPrefix class |
java/ql/lib/semmle/code/java/security/StaticInitializationVectorQuery.qll |
Added local overlay annotation to ArrayUpdate class |
java/ql/lib/semmle/code/java/security/BrokenCryptoAlgorithmQuery.qll |
Added local overlay annotation to BrokenAlgoLiteral class |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you run a DCA overlay experiment to test if these changes had any impact on overlay accuracy for these queries?
ma = this and | ||
ma.getArgument(0) = array | ||
| | ||
m.getAnOverride*().hasQualifiedName("java.io", ["InputStream", "RandomAccessFile"], "read") or |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These m.getAnOverride*()
aren't file-local though they seem unlikely to matter in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point as these are getAnOverride
so point backwards to what I thought they did.
I have a DCA experiment here; https://github.com/github/codeql-dca-main/issues/32115 . it's in progress as it failed the first time. |
This makes some expensive predicates in "query" libraries local. These all seem obviously local to me and they are all slow (mostly due to regexes, but soemtiems for other reasons)