-
Notifications
You must be signed in to change notification settings - Fork 329
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
Respect scaling_reserved_ram
feature flag
#1760
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.
Looks good. It's perhaps a little cautious, but we might want to add an integration test that runs the CodeQL Action with this feature enabled. To do so, we could make a copy of pr-checks/checks/multi-language-autodetect.yml
, enable the feature flag via the environment variable, and remove the language autodetection checks.
Oh, do add a changelog note too! The one from 24th May is a good example. |
The amount of RAM given to the CodeQL evaluator is the machine's total memory size, minus a reserved amount. Currently, the reserved amount is fixed at 1 GB (or 1.5 GB on Windows). When the scaling_reserved_ram feature flag is enabled, we also add 2% of the total memory size to the reserved amount. This allows for the fact that the kernel will consume more RAM (e.g. for page tables) on machines with more physical RAM.
e392b54
to
6b7e394
Compare
6b7e394
to
ab9aa50
Compare
I see the new integration test is running with |
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.
Excellent! (I think you meant the multi-language autodetection test runs with --ram=5907
.)
Whoops, yes, that's what I meant. |
The amount of RAM given to the CodeQL evaluator is the machine's total memory size, minus a reserved amount.
Currently, the reserved amount is fixed at 1 GB (or 1.5 GB on Windows). When the
scaling_reserved_ram
feature flag is enabled, we also add 2% of the total memory size to the reserved amount. This allows for the fact that the kernel will consume more RAM (e.g. for page tables) on machines with more physical RAM, so we should see fewer analyses getting killed by the OOM killer.N.B. This is my first time working on the Action, or even writing TypeScript. I hope my uses of
async
/await
make sense.Merge / deployment checklist