Skip to content
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

Java: Adjust caching of BasicBlocks, BaseSSA, and CompileTimeConstants #19093

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

aschackmull
Copy link
Contributor

@aschackmull aschackmull commented Mar 21, 2025

This PR makes 3 changes to caching in the Java QL libs (separated cleanly in individual commits):

  • Merge the cached stages related to BasicBlocks, and cache the char-pred.
  • Merge the cached stages related to BaseSSA, and cache the SourceVariable newtype.
  • Values of CompileTimeConstants of type int, bool, and string involve mutually recursive calculation. The int values were cached but the other two weren't, so I fixed that. Since they're mutually recursive they all end up in the same stage.

All 3 of these changes were motivated by statically inspecting DIL overlap between stages. These cases stood out as potentially being able to cause recomputation.

The changes caused a bit of performance fallout, which was fixed in an additional commit.

@github-actions github-actions bot added the Java label Mar 21, 2025
@@ -24,7 +40,10 @@

/** Gets an immediate successor of this basic block. */
cached
BasicBlock getABBSuccessor() { result = this.getLastNode().getASuccessor() }
BasicBlock getABBSuccessor() {

Check warning

Code scanning / CodeQL

Acronyms should be PascalCase/camelCase. Warning

Acronyms in getABBSuccessor should be PascalCase/camelCase.
@aschackmull aschackmull added the no-change-note-required This PR does not need a change note label Mar 25, 2025
@aschackmull aschackmull changed the title Java: Adjust caching Java: Adjust caching of BasicBlocks, BaseSSA, and CompileTimeConstants Mar 25, 2025
@aschackmull
Copy link
Contributor Author

Dca looks like a decent performance improvement.

@aschackmull aschackmull marked this pull request as ready for review March 25, 2025 08:17
@aschackmull aschackmull requested a review from a team as a code owner March 25, 2025 08:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Java 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.

1 participant