Skip to content

Commit

Permalink
Merge pull request #1796 from github/henrymercer/scaling-ram-larger-r…
Browse files Browse the repository at this point in the history
…unners-only

Scale the amount of reserved RAM on large runners only
  • Loading branch information
henrymercer committed Jul 24, 2023
2 parents ce84bed + fda93d8 commit 7b6664f
Show file tree
Hide file tree
Showing 6 changed files with 172 additions and 54 deletions.
26 changes: 18 additions & 8 deletions lib/util.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/util.js.map

Large diffs are not rendered by default.

71 changes: 53 additions & 18 deletions lib/util.test.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion lib/util.test.js.map

Large diffs are not rendered by default.

93 changes: 72 additions & 21 deletions src/util.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,79 @@ test("getToolNames", (t) => {
t.deepEqual(toolNames, ["CodeQL command-line toolchain", "ESLint"]);
});

test("getMemoryFlag() should return the correct --ram flag", async (t) => {
const totalMem = os.totalmem() / (1024 * 1024);
const fixedAmount = process.platform === "win32" ? 1536 : 1024;
const scaledAmount = 0.02 * totalMem;
const expectedMemoryValue = Math.floor(totalMem - fixedAmount);
const expectedMemoryValueWithScaling = Math.floor(
totalMem - fixedAmount - scaledAmount
const GET_MEMORY_FLAG_TESTS = [
{
input: undefined,
totalMemoryMb: 8 * 1024,
platform: "linux",
expectedMemoryValue: 7 * 1024,
expectedMemoryValueWithScaling: 7 * 1024,
},
{
input: undefined,
totalMemoryMb: 8 * 1024,
platform: "win32",
expectedMemoryValue: 6.5 * 1024,
expectedMemoryValueWithScaling: 6.5 * 1024,
},
{
input: "",
totalMemoryMb: 8 * 1024,
platform: "linux",
expectedMemoryValue: 7 * 1024,
expectedMemoryValueWithScaling: 7 * 1024,
},
{
input: "512",
totalMemoryMb: 8 * 1024,
platform: "linux",
expectedMemoryValue: 512,
expectedMemoryValueWithScaling: 512,
},
{
input: undefined,
totalMemoryMb: 64 * 1024,
platform: "linux",
expectedMemoryValue: 63 * 1024,
expectedMemoryValueWithScaling: 63078, // Math.floor(1024 * (64 - 1 - 0.025 * (64 - 8)))
},
{
input: undefined,
totalMemoryMb: 64 * 1024,
platform: "win32",
expectedMemoryValue: 62.5 * 1024,
expectedMemoryValueWithScaling: 62566, // Math.floor(1024 * (64 - 1.5 - 0.025 * (64 - 8)))
},
];

for (const {
input,
totalMemoryMb,
platform,
expectedMemoryValue,
expectedMemoryValueWithScaling,
} of GET_MEMORY_FLAG_TESTS) {
test(
`Memory flag value is ${expectedMemoryValue} without scaling and ${expectedMemoryValueWithScaling} with scaling ` +
`for ${
input ?? "no user input"
} on ${platform} with ${totalMemoryMb} MB total system RAM`,
async (t) => {
for (const withScaling of [true, false]) {
const flag = util.getMemoryFlagValueForPlatform(
input,
totalMemoryMb * 1024 * 1024,
platform,
withScaling
);
t.deepEqual(
flag,
withScaling ? expectedMemoryValueWithScaling : expectedMemoryValue
);
}
}
);

const tests: Array<[string | undefined, boolean, string]> = [
[undefined, false, `--ram=${expectedMemoryValue}`],
["", false, `--ram=${expectedMemoryValue}`],
["512", false, "--ram=512"],
[undefined, true, `--ram=${expectedMemoryValueWithScaling}`],
["", true, `--ram=${expectedMemoryValueWithScaling}`],
];

for (const [input, withScaling, expectedFlag] of tests) {
const flag = util.getMemoryFlag(input, withScaling);
t.deepEqual(flag, expectedFlag);
}
});
}

test("getMemoryFlag() throws if the ram input is < 0 or NaN", async (t) => {
for (const input of ["-1", "hello!"]) {
Expand Down
32 changes: 27 additions & 5 deletions src/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,15 +153,16 @@ export async function withTmpDir<T>(
*/
function getSystemReservedMemoryMegaBytes(
totalMemoryMegaBytes: number,
platform: string,
isScalingReservedRamEnabled: boolean
): number {
// Windows needs more memory for OS processes.
const fixedAmount = 1024 * (process.platform === "win32" ? 1.5 : 1);
const fixedAmount = 1024 * (platform === "win32" ? 1.5 : 1);

if (isScalingReservedRamEnabled) {
// Reserve an additional 2% of the total memory, since the amount used by
// Reserve an additional 2.5% of the amount of memory above 8 GB, since the amount used by
// the kernel for page tables scales with the size of physical memory.
const scaledAmount = 0.02 * totalMemoryMegaBytes;
const scaledAmount = 0.025 * Math.max(totalMemoryMegaBytes - 8 * 1024, 0);
return fixedAmount + scaledAmount;
} else {
return fixedAmount;
Expand All @@ -175,8 +176,10 @@ function getSystemReservedMemoryMegaBytes(
*
* @returns {number} the amount of RAM to use, in megabytes
*/
export function getMemoryFlagValue(
export function getMemoryFlagValueForPlatform(
userInput: string | undefined,
totalMemoryBytes: number,
platform: string,
isScalingReservedRamEnabled: boolean
): number {
let memoryToUseMegaBytes: number;
Expand All @@ -186,17 +189,36 @@ export function getMemoryFlagValue(
throw new Error(`Invalid RAM setting "${userInput}", specified.`);
}
} else {
const totalMemoryBytes = os.totalmem();
const totalMemoryMegaBytes = totalMemoryBytes / (1024 * 1024);
const reservedMemoryMegaBytes = getSystemReservedMemoryMegaBytes(
totalMemoryMegaBytes,
platform,
isScalingReservedRamEnabled
);
memoryToUseMegaBytes = totalMemoryMegaBytes - reservedMemoryMegaBytes;
}
return Math.floor(memoryToUseMegaBytes);
}

/**
* Get the value of the codeql `--ram` flag as configured by the `ram` input.
* If no value was specified, the total available memory will be used minus a
* threshold reserved for the OS.
*
* @returns {number} the amount of RAM to use, in megabytes
*/
export function getMemoryFlagValue(
userInput: string | undefined,
isScalingReservedRamEnabled: boolean
): number {
return getMemoryFlagValueForPlatform(
userInput,
os.totalmem(),
process.platform,
isScalingReservedRamEnabled
);
}

/**
* Get the codeql `--ram` flag as configured by the `ram` input. If no value was
* specified, the total available memory will be used minus a threshold
Expand Down

0 comments on commit 7b6664f

Please sign in to comment.