Set --expect-discarded-cache option #1540
Conversation
This refactoring commit changes databaseRunQueries() to accept a list of flags instead of separate memory and threads flags.
aeisenberg
left a comment
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.
src/analyze.ts
Outdated
| 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?
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).
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! |
| }); | ||
| }); | ||
|
|
||
| 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).
2 participants
This PR changes the action to pass
--expect-discarded-cacheto the lastcodeql database run-queriesinvocation to improve performance.Merge / deployment checklist