Skip to content

Conversation

@Abhinav-Prajapati
Copy link

Overview

This PR introduces a memory tracking feature for all programs by leveraging the /usr/bin/time command. The goal is to monitor and validate how much memory each programming language is using during execution, and to align memory usage reporting across languages (C, Python, Node.js, etc.).


Changes

Memory Tracking Feature

  • Implemented memory usage tracking using /usr/bin/time for accurate and consistent memory measurement across languages.

Expanded Test Cases

  • Added and updated C test cases in testJson.js to cover:

    • Heap Allocation Test: Allocates 50MB on the heap, touches the memory, then frees it. Verifies both output and memory usage.
    • Stack Allocation Test: Recursively allocates 1MB per call to reach 6MB of stack usage, ensuring it remains within safe system limits.

Request Body

{
  "language": "c",
  "script": "#include <stdio.h>\n#include <string.h>\n\n#define ONE_MB (1024 * 1024)\n\nvoid stack_allocate(int remaining_bytes, int depth) {\n    if (remaining_bytes <= 0) {\n        printf(\"Memory allocated on stack\\n\");\n        return;\n    }\n    char buffer[ONE_MB];\n    memset(buffer, 0, ONE_MB);  // Touch the memory\n    stack_allocate(remaining_bytes - ONE_MB, depth + 1);\n}\n\nint main() {\n    stack_allocate(6 * ONE_MB, 0);\n    return 0;\n}\n"
}

API Response Output:

{
  "output": "Memory allocated on stack\n",
  "execute_time": null,
  "errorMessage": "",
  "status_code": 200,
  "memory": 6544,
  "cpu_time": null,
  "output_files": [],
  "compile_message": "",
  "error": 0
}
  • Introduced an approxMemoryUses field for test cases to define expected memory ranges.

Code Improvements

  • Updated all test case program strings to use template literals (backticks) instead of string concatenation for improved readability and maintainability.

Documentation

  • Added a new Markdown file in the doc/ directory explaining:

    • The rationale behind choosing /usr/bin/time over Valgrind.

await fs.promises.access(path, fs.constants.F_OK) // Check if file exists
const memoryReport = await fs.promises.readFile(path, 'utf8')
const memoryMatch = memoryReport.match(
/Maximum resident set size \(kbytes\):\s*(\d+)/,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

instead of this, can you just make time only output the resident size. I think you can provide -f to time as argument.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

made changes as suggested

' }\n' +
' return 0;\n\n' +
'}\n',
script: `#include<bits/stdc++.h>

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add newline in all the first lines, so the code looks better.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Refactored as suggested

@anilgulecha
Copy link

/gemini review

Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request successfully introduces memory usage tracking using /usr/bin/time and updates the test suite to include memory assertions, particularly for C programs testing heap and stack allocation. The conversion of script strings to template literals improves readability. The documentation explaining the choice of /usr/bin/time is a valuable addition. The memory unit handling based on potential Alpine page size is a potential point of fragility that should be addressed for robustness across different environments. The formatting of the template literals in the test data could also be slightly cleaner.

}

// Adjust if you're on Alpine to divide by 4
response.memory = memoryKB ? memoryKB / 4 : null
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The comment suggests dividing the memory usage by 4 if running on Alpine. However, /usr/bin/time -f "%M" is documented to report the maximum resident set size in Kilobytes on Linux systems. Dividing by 4 implies the output is in 4KB pages, which is not the standard behavior of %M. Relying on this division is fragile and could lead to incorrect memory reporting if the execution environment changes or if /usr/bin/time behaves as documented (reporting in KB). It's best to remove this division unless there's a specific, confirmed reason for it on the target platform that overrides the standard documentation.

Suggested change
response.memory = memoryKB ? memoryKB / 4 : null
response.memory = memoryKB ? memoryKB : null

Comment on lines +6 to +12
script: `
\n#include<bits/stdc++.h>
\nusing namespace std;
\nint main(){
\n cout << "hello world";
\n return 0;
\n}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While using template literals is a good improvement over string concatenation, the inclusion of leading \n characters and the inconsistent indentation within the template literal make the embedded code less readable. Consider removing the leading \n and adjusting the indentation so the code inside the backticks is formatted cleanly.

Suggested change
script: `
\n#include<bits/stdc++.h>
\nusing namespace std;
\nint main(){
\n cout << "hello world";
\n return 0;
\n}`,
script: `#include<bits/stdc++.h>
using namespace std;
int main(){
cout << "hello world";
return 0;
}`,

Comment on lines +25 to +34
script: `
\n#include<bits/stdc++.h>
\nusing namespace std;
\nint main(){
\nint a;
\nwhile(cin >> a){
\ncout << a << endl;
\n}
\nreturn 0;
\n}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the previous comment, the formatting of the script within the template literal could be improved for readability by removing the leading \n and adjusting indentation.

Suggested change
script: `
\n#include<bits/stdc++.h>
\nusing namespace std;
\nint main(){
\nint a;
\nwhile(cin >> a){
\ncout << a << endl;
\n}
\nreturn 0;
\n}`,
script: `#include<bits/stdc++.h>
using namespace std;
int main(){
int a;
while(cin >> a){
cout << a << endl;
}
return 0;
}`,

Comment on lines +63 to +67
script: `
\nprocess.stdin.setEncoding('utf8');
\nprocess.stdin.on('data', (input) => {
\nconsole.log(input);
\n});`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The template literal formatting includes unnecessary leading \n and inconsistent indentation. Clean up the formatting for better readability.

Suggested change
script: `
\nprocess.stdin.setEncoding('utf8');
\nprocess.stdin.on('data', (input) => {
\nconsole.log(input);
\n});`,
script: `process.stdin.setEncoding('utf8');
process.stdin.on('data', (input) => {
console.log(input);
});`,

Comment on lines +96 to +103
`\ntry:
\n while(True):
\n line = input()
\n if not line:
\n break
\n print(line)
\nexcept EOFError:
\n pass`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Clean up the template literal formatting by removing the leading \n and adjusting indentation.

            script: `try:
    while(True):
        line = input()
        if not line:
            break
        print(line)
except EOFError:
    pass`,

Comment on lines +219 to +220
`\nuser_input = gets.chomp
\nputs user_input`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Clean up the template literal formatting by removing the leading \n and adjusting indentation.

            script: `user_input = gets.chomp
puts user_input`,

Comment on lines +260 to +272
script: `
\nimport time
\ndef consume_memory(target_mb, duration_sec):
\n float_size = 8
\n floats_per_mb = (1024 * 1024) // float_size
\n total_floats = target_mb * floats_per_mb
\n iterations = int(duration_sec / 0.1)
\n floats_per_iteration = total_floats // iterations
\n memory_hog = []
\n for _ in range(iterations):
\n memory_hog.extend([0.0] * floats_per_iteration)
\n time.sleep(0.1)
\nconsume_memory(1000, 1)`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Clean up the template literal formatting by removing the leading \n and adjusting indentation.

            script: `import time
def consume_memory(target_mb, duration_sec):
    float_size = 8
    floats_per_mb = (1024 * 1024) // float_size
    total_floats = target_mb * floats_per_mb
    iterations = int(duration_sec / 0.1)
    floats_per_iteration = total_floats // iterations
    memory_hog = []
    for _ in range(iterations):
        memory_hog.extend([0.0] * floats_per_iteration)
        time.sleep(0.1)
consume_memory(1000, 1)`,

Comment on lines +285 to +288
`\na = [100]
\nfor i in a:
\n a.append(i)
\n`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Clean up the template literal formatting by removing the leading \n and adjusting indentation.

            script: `a = [100]
for i in a:
    a.append(i)
`,

Comment on lines +325 to +342
`\n#include <stdio.h>
\n#include <stdlib.h>
\nint main() {
\n size_t memory_size = 50 * 1024 * 1024;
\n char *memory_block = malloc(memory_size);
\n if (memory_block == NULL) {
\n printf("Failed to allocate memory\\n");
\n return 1;
\n }
\n printf("Memory allocation done\\n");
\n for (size_t i = 0; i < memory_size; i += 4096) {
\n memory_block[i] = (char)(i % 256);
\n }
\n printf("Memory touched and initialized\\n");
\n free(memory_block);
\n printf("Memory freed\\n");
\n return 0;
\n}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Clean up the template literal formatting by removing the leading \n and adjusting indentation.

            script: `#include <stdio.h>
#include <stdlib.h>
int main() {
    size_t memory_size = 50 * 1024 * 1024;
    char *memory_block = malloc(memory_size);
    if (memory_block == NULL) {
        printf("Failed to allocate memory\\n");
        return 1;
    }
    printf("Memory allocation done\\n");
    for (size_t i = 0; i < memory_size; i += 4096) {
        memory_block[i] = (char)(i % 256);
    }
    printf("Memory touched and initialized\\n");
    free(memory_block);
    printf("Memory freed\\n");
    return 0;
}`,

Comment on lines +356 to +371
`\n#include <stdio.h>
\n#include <string.h>
\n#define ONE_MB (1024 * 1024)
\nvoid stack_allocate(int remaining_bytes, int depth) {
\n if (remaining_bytes <= 0) {
\n printf("Memory allocated on stack\\n");
\n return;
\n }
\n char buffer[ONE_MB];
\n memset(buffer, 0, ONE_MB); // Touch the memory
\n stack_allocate(remaining_bytes - ONE_MB, depth + 1);
\n}
\nint main() {
\n stack_allocate(6 * ONE_MB, 0);
\n return 0;
\n}`,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Clean up the template literal formatting by removing the leading \n and adjusting indentation.

            script: `#include <stdio.h>
#include <string.h>
#define ONE_MB (1024 * 1024)
void stack_allocate(int remaining_bytes, int depth) {
    if (remaining_bytes <= 0) {
        printf("Memory allocated on stack\\n");
        return;
    }
    char buffer[ONE_MB];
    memset(buffer, 0, ONE_MB);  // Touch the memory
    stack_allocate(remaining_bytes - ONE_MB, depth + 1);
}
int main() {
    stack_allocate(6 * ONE_MB, 0);
    return 0;
}`,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants