Set --expect-discarded-cache option - #1540
cklin merged 5 commits into
Conversation
Sorry, something went wrong.
This refactoring commit changes databaseRunQueries() to accept a list of flags instead of separate memory and threads flags.
aeisenberg
left a comment
There was a problem hiding this comment.
A question inline.
Also, can you think of any way of testing this? Probably a unit test or two would be fine. We are testing the runQueries command in this test: https://github.com/aeisenberg/codeql-action/blob/main/src/analyze.test.ts#L25 This test could be expanded, or a new test that focuses explicitly on this flag could be written.
We can pair on this if you like.
Sorry, something went wrong.
| const isLastLanguage = | ||
| language === config.languages[config.languages.length - 1]; |
There was a problem hiding this comment.
Not sure how --expect-discarded-cache actually works, but does it affect all languages in a cluster at the same time? Ie- if you have a clustered db with java and javascript, will running with --expect-discarded-cache on java queries affect the cache for javascript?
Sorry, something went wrong.
There was a problem hiding this comment.
Based on discussions in the tracking issue, I think we should set --expect-discarded-cache for the last run-queries command of each language (as opposed to for all languages).
Sorry, something went wrong.
This refactoring commit changes runQueries() to calculate the set of indices with non-empty custom queries early. Doing so allows us to check early on whether there are any custom queries to run.
|
I made the suggested changes and added unit tests. PTAL. Thank you! |
Sorry, something went wrong.
aeisenberg
left a comment
There was a problem hiding this comment.
Nice work.
Sorry, something went wrong.
| }); | ||
| }); | ||
|
|
||
| test("optimizeForLastQueryRun for two languages, CliConfigFileEnabled", async (t) => { |
There was a problem hiding this comment.
Going forward, Feature.CliConfigFileEnabled should be enabled for all users (except some specific DCA repos).
Sorry, something went wrong.
This PR changes the action to pass --expect-discarded-cache to the last codeql database run-queries invocation to improve performance.
Merge / deployment checklist