-
Notifications
You must be signed in to change notification settings - Fork 385
Add RAM and threads options to init action #738
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -18,7 +18,12 @@ inputs: | |
required: false | ||
default: "brutal" | ||
ram: | ||
description: Override the amount of memory in MB to be used by CodeQL. By default, almost all the memory of the machine is used. | ||
description: >- | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the input descriptions here are going to vary between the two actions, we should add an exception to the custom query that's producing the code scanning warning on this file. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I edited |
||
The amount of memory in MB that can be used by CodeQL for database finalization and query execution. | ||
By default, this action will use the same amount of memory as previously set in the "init" action. | ||
adityasharad marked this conversation as resolved.
Show resolved
Hide resolved
|
||
If the "init" action also does not have an explicit "ram" input, this action will use most of the | ||
memory available in the system (which for GitHub-hosted runners is 6GB for Linux, 5.5GB for Windows, | ||
and 13GB for macOS). | ||
required: false | ||
add-snippets: | ||
description: Specify whether or not to add code snippets to the output sarif file. | ||
|
@@ -29,7 +34,12 @@ inputs: | |
required: false | ||
default: "false" | ||
threads: | ||
description: The number of threads to be used by CodeQL. | ||
description: >- | ||
The number of threads that can be used by CodeQL for database finalization and query execution. | ||
By default, this action will use the same number of threads as previously set in the "init" action. | ||
adityasharad marked this conversation as resolved.
Show resolved
Hide resolved
|
||
If the "init" action also does not have an explicit "threads" input, this action will use all the | ||
hardware threads available in the system (which for GitHub-hosted runners is 2 for Linux and Windows | ||
and 3 for macOS). | ||
required: false | ||
checkout_path: | ||
description: "The path at which the analyzed repository was checked out. Used to relativize any absolute paths in the uploaded SARIF file." | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
I think I'd slightly prefer this to use something other than
1
for the threads as1
could very easily be a default in which case we wouldn't really be checking anything. Maybe3
as that seems very unlikely to ever be the default value.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.
This is a decision that I struggled with.
The main challenge here is that the number of threads used defaults to (and is capped at) the number of CPU cores available on the machine. GitHub action runners have 2 CPUs, so both the default and the cap would be 2. Passing any value larger than 1 means that the test will break if it runs on a system with different number of CPUs.
I would love to hear your thoughts. For example, maybe we could assume that the runner will always have at least 2 CPUs and have two tests with threads set to 1 and 2?
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.
Aha, this is a good point. As I mentioned below, I'm actually not sure that capping this is a good idea, but for now let's leave this test as is 🙂