Merge pull request #396 from github/adityasharad/ram-threshold

Increase the default amount of RAM reserved for the OS

lib/util.js

@@ -72,9 +72,21 @@ async function withTmpDir(body) {
     return result;
 }
 exports.withTmpDir = withTmpDir;
+/**
+ * Gets an OS-specific amount of memory (in MB) to reserve for OS processes
+ * when the user doesn't explicitly specify a memory setting.
+ * This is a heuristic to avoid OOM errors (exit code 137 / SIGKILL)
+ * from committing too much of the available memory to CodeQL.
+ * @returns number
+ */
+function getSystemReservedMemoryMegaBytes() {
+    // Windows needs more memory for OS processes.
+    return 1024 * (process.platform === "win32" ? 1.5 : 1);
+}
 /**
  * Get the codeql `--ram` flag as configured by the `ram` input. If no value was
- * specified, the total available memory will be used minus 256 MB.
+ * specified, the total available memory will be used minus a threshold
+ * reserved for the OS.
  *
  * @returns string
  */
@@ -89,8 +101,8 @@ function getMemoryFlag(userInput) {
     else {
         const totalMemoryBytes = os.totalmem();
         const totalMemoryMegaBytes = totalMemoryBytes / (1024 * 1024);
-        const systemReservedMemoryMegaBytes = 256;
+        const reservedMemoryMegaBytes = getSystemReservedMemoryMegaBytes();
-        memoryToUseMegaBytes = totalMemoryMegaBytes - systemReservedMemoryMegaBytes;
+        memoryToUseMegaBytes = totalMemoryMegaBytes - reservedMemoryMegaBytes;
     }
     return `--ram=${Math.floor(memoryToUseMegaBytes)}`;
 }

lib/util.test.js

@@ -28,9 +28,10 @@ ava_1.default("getToolNames", (t) => {
 });
 ava_1.default("getMemoryFlag() should return the correct --ram flag", (t) => {
     const totalMem = Math.floor(os.totalmem() / (1024 * 1024));
+    const expectedThreshold = process.platform === "win32" ? 1536 : 1024;
     const tests = [
-        [undefined, `--ram=${totalMem - 256}`],
+        [undefined, `--ram=${totalMem - expectedThreshold}`],
-        ["", `--ram=${totalMem - 256}`],
+        ["", `--ram=${totalMem - expectedThreshold}`],
         ["512", "--ram=512"],
     ];
     for (const [input, expectedFlag] of tests) {

src/util.test.ts

@@ -24,10 +24,11 @@ test("getToolNames", (t) => {
 
 test("getMemoryFlag() should return the correct --ram flag", (t) => {
   const totalMem = Math.floor(os.totalmem() / (1024 * 1024));
+  const expectedThreshold = process.platform === "win32" ? 1536 : 1024;
 
   const tests = [
-    [undefined, `--ram=${totalMem - 256}`],
+    [undefined, `--ram=${totalMem - expectedThreshold}`],
-    ["", `--ram=${totalMem - 256}`],
+    ["", `--ram=${totalMem - expectedThreshold}`],
     ["512", "--ram=512"],
   ];
 

src/util.ts

@@ -82,9 +82,22 @@ export async function withTmpDir<T>(
   return result;
 }
 
+/**
+ * Gets an OS-specific amount of memory (in MB) to reserve for OS processes
+ * when the user doesn't explicitly specify a memory setting.
+ * This is a heuristic to avoid OOM errors (exit code 137 / SIGKILL)
+ * from committing too much of the available memory to CodeQL.
+ * @returns number
+ */
+function getSystemReservedMemoryMegaBytes(): number {
+  // Windows needs more memory for OS processes.
+  return 1024 * (process.platform === "win32" ? 1.5 : 1);
+}
+
 /**
  * Get the codeql `--ram` flag as configured by the `ram` input. If no value was
- * specified, the total available memory will be used minus 256 MB.
+ * specified, the total available memory will be used minus a threshold
+ * reserved for the OS.
  *
  * @returns string
  */
@@ -98,8 +111,8 @@ export function getMemoryFlag(userInput: string | undefined): string {
   } else {
     const totalMemoryBytes = os.totalmem();
     const totalMemoryMegaBytes = totalMemoryBytes / (1024 * 1024);
-    const systemReservedMemoryMegaBytes = 256;
+    const reservedMemoryMegaBytes = getSystemReservedMemoryMegaBytes();
-    memoryToUseMegaBytes = totalMemoryMegaBytes - systemReservedMemoryMegaBytes;
+    memoryToUseMegaBytes = totalMemoryMegaBytes - reservedMemoryMegaBytes;
   }
   return `--ram=${Math.floor(memoryToUseMegaBytes)}`;
 }