Use the full amount of memory when running queries. #37
Conversation
robertbrignull
left a comment
robertbrignull
left a comment
There was a problem hiding this comment.
LGTM
How can we tell if it's working? There should be projects that don't build now but do with this change, but I'm unsure how to find one to test with. Queries should theoretically be a bit faster too, but I'm not sure if it'll be a noticeable amount.
src/finalize-db.ts
Outdated
| const memoryToUseString = core.getInput("ram"); | ||
| if (memoryToUseString) { | ||
| memoryToUseMegaBytes = Number(memoryToUseString); | ||
| if (Number.isNaN(memoryToUseMegaBytes)) { |
There was a problem hiding this comment.
Probably worth adding a check that it's greater than 0 too.
| if (Number.isNaN(memoryToUseMegaBytes)) { | |
| if (Number.isNaN(memoryToUseMegaBytes) || memoryToUseMegaBytes <= 0) { |
|
The way I tested it was just to run Before this change you get: And afterwards it is: There should also be projects that were too big to analyze that now work, but I also don't know any examples off the top of my head. |
Currently we call CodeQL without specifying
--ram. This means the amount of memory it uses is whatever Java happens to decide. In the case of the Actions workers that seems to be 1732MB. The actions workers have a ton more memory than this though, and since Actions steps are generally run serially we can use almost all of it if we want to.This change adds automatic detection of how much memory to use based on the total system memory minus 256MB for other processes that might be running. I've also added an input that allows overriding this, but I don't think most people will need it so I've left it out of the documentation for now.
Merge / deployment checklist