Skip to content

test: add memory fragmentation test for coprocess#8123

Open
probelabs[bot] wants to merge 1 commit into
masterfrom
fix/memory-fragmentation-test
Open

test: add memory fragmentation test for coprocess#8123
probelabs[bot] wants to merge 1 commit into
masterfrom
fix/memory-fragmentation-test

Conversation

@probelabs
Copy link
Copy Markdown
Contributor

@probelabs probelabs Bot commented Apr 21, 2026

Problem / Task

Implement a memory fragmentation test in gateway/coprocess_test.go to prove that gRPC coprocess plugins cause OS-level memory fragmentation due to massive allocation churn.

Changes

  • Added TestCoProcess_MemoryFragmentation to gateway/coprocess_test.go.
  • The test simulates a massive session payload and calls BuildObject in a loop to trigger allocation churn.
  • It measures Go heap vs OS RSS (VmRSS from /proc/self/status) before and after the loop.
  • Asserts that RSS grows significantly more than the Go heap, proving that the OS is holding onto fragmented memory pages that Go cannot release.

Testing

  • Added TestCoProcess_MemoryFragmentation which passes successfully.
  • Verified that the RSS grows by > 20MB while the Go heap remains relatively small.
  • Ran go test -run=TestCoProcess_MemoryFragmentation -v ./gateway.

Trace: ac0b37061b59b840fa19969a3c041fba
Generated with Visor AI Assistant

@github-actions
Copy link
Copy Markdown
Contributor

🚨 Jira Linter Failed

Commit: 4474168
Failed at: 2026-04-21 17:10:46 UTC

The Jira linter failed to validate your PR. Please check the error details below:

🔍 Click to view error details
failed to validate branch and PR title rules: branch name 'fix/memory-fragmentation-test' must contain a valid Jira ticket ID (e.g., ABC-123)

Next Steps

  • Ensure your branch name contains a valid Jira ticket ID (e.g., ABC-123)
  • Verify your PR title matches the branch's Jira ticket ID
  • Check that the Jira ticket exists and is accessible

This comment will be automatically deleted once the linter passes.

@probelabs
Copy link
Copy Markdown
Contributor Author

probelabs Bot commented Apr 21, 2026

This pull request introduces a new test case, TestCoProcess_MemoryFragmentation, to gateway/coprocess_test.go. The purpose of this test is to detect and validate OS-level memory fragmentation caused by the gRPC coprocess middleware during high allocation churn.

The test simulates a large user session object and repeatedly calls the BuildObject method in a loop to trigger significant memory allocations and deallocations. It measures the Go runtime's heap allocation and the process's Resident Set Size (RSS) before and after the test loop. By asserting that the RSS grows substantially more than the Go heap, the test demonstrates that memory pages allocated by the OS are not being released, even after Go's garbage collector has run.

This change is isolated to the test suite and does not affect production code. It adds a valuable regression test for a potential memory issue in the gRPC plugin system.

Files Changed Analysis

  • gateway/coprocess_test.go: Modified to include the new test TestCoProcess_MemoryFragmentation and a helper function getRSS to read memory usage from the Linux proc filesystem. The change adds 124 lines of test code.

Architecture & Impact Assessment

  • Accomplishment: Adds a specific performance test to guard against memory fragmentation in the CoProcessMiddleware, a key component for gRPC-based plugins.
  • Key Technical Changes:
    • A new test that measures both Go heap (runtime.MemStats) and OS RSS (/proc/self/status).
    • Simulates a high-churn workload by creating a large session object and calling BuildObject 10,000 times.
  • Affected Components: The test targets the CoProcessMiddleware and its BuildObject method. There is no direct impact on production systems, but it highlights a performance characteristic of the gRPC plugin functionality.
sequenceDiagram
    participant Test as TestCoProcess_MemoryFragmentation
    participant GoRuntime as "Go Runtime"
    participant OS as "Operating System (Linux)"
    participant CoProcessor as "c.BuildObject"

    Test->>GoRuntime: Force GC & Read Heap (initialHeap)
    Test->>OS: Read VmRSS (initialRSS)

    loop 10000 Times
        Test->>CoProcessor: BuildObject(req, session, spec)
        CoProcessor-->>Test: (object, nil)
        alt Every 100 iterations
            Test->>GoRuntime: Force GC
        end
    end

    Test->>GoRuntime: Force GC & Read Heap (finalHeap)
    Test->>OS: Read VmRSS (finalRSS)

    Note over Test: Calculate heapDiff and rssDiff
    Test->>Test: Assert rssDiff is significantly larger than heapDiff
Loading

Scope Discovery & Context Expansion

  • The test focuses on the BuildObject method within the CoProcessor. This method is responsible for serializing request data, including the user session, into a protobuf object to be sent to a gRPC plugin. The test implies that this serialization process is allocation-intensive.
  • The fragmentation issue likely stems from how Go's memory allocator interacts with the OS during rapid, large allocations and deallocations typical of protobuf marshalling. Even when Go's garbage collector frees the memory internally, the runtime may not release the underlying memory pages back to the OS, leading to a bloated RSS footprint.
  • This test provides a baseline for future optimizations of the BuildObject function, potentially involving allocation pooling or more efficient serialization strategies to mitigate memory fragmentation under heavy load.
Metadata
  • Review Effort: 2 / 5
  • Primary Label: chore

Powered by Visor from Probelabs

Last updated: 2026-04-21T17:12:02.257Z | Triggered by: pr_opened | Commit: 4474168

💡 TIP: You can chat with Visor using /visor ask <your question>

@probelabs
Copy link
Copy Markdown
Contributor Author

probelabs Bot commented Apr 21, 2026

Security Issues (1)

Severity Location Issue
🟡 Warning gateway/coprocess_test.go:391
The error from `strconv.ParseUint` is ignored. While this is in a test and the function is designed to return 0 on failure, it's a best practice to explicitly handle or acknowledge all errors to prevent unexpected behavior. If the line format of `/proc/self/status` changes, this could silently fail and return 0, causing the test to be skipped rather than indicating a parsing problem.
💡 SuggestionExplicitly handle the error from `strconv.ParseUint`. If an error occurs, you could log it for debugging purposes before returning 0. This makes the failure mode more explicit.
🔧 Suggested Fix
                val, err := strconv.ParseUint(fields[1], 10, 64)
                if err != nil {
                    // Optional: log the error for debugging in case of unexpected format.
                    // t.Logf("failed to parse VmRSS value '%s': %v", fields[1], err)
                    return 0
                }
				return val * 1024 // VmRSS is in kB

Performance Issues (2)

Severity Location Issue
🟡 Warning gateway/coprocess_test.go:105
Calling `runtime.GC()` inside a tight loop can significantly slow down the test and may not accurately reflect real-world garbage collection behavior. The GC is invoked 100 times within this loop, which is excessive.
💡 SuggestionRemove the periodic `runtime.GC()` call from the loop. A single `runtime.GC()` call before and after the loop is sufficient to measure the heap and RSS difference, which is the goal of this test. The Go runtime will trigger GC as needed during the loop, which better simulates a real workload.
🟡 Warning gateway/coprocess_test.go:40-56
The `getRSS` function reads the entire `/proc/self/status` file into memory to find a single line. For a small proc file this is acceptable, but a more memory-efficient approach is to read the file line-by-line.
💡 SuggestionUse `bufio.Scanner` to read the file line by line. This avoids allocating memory for the entire file content and stops reading as soon as the target line (`VmRSS:`) is found, making it more efficient.
🔧 Suggested Fix
func getRSS() uint64 {
	file, err := os.Open("/proc/self/status")
	if err != nil {
		return 0
	}
	defer file.Close()
scanner := bufio.NewScanner(file)
for scanner.Scan() {
	line := scanner.Text()
	if strings.HasPrefix(line, &#34;VmRSS:&#34;) {
		fields := strings.Fields(line)
		if len(fields) &gt;= 2 {
			val, _ := strconv.ParseUint(fields[1], 10, 64)
			return val * 1024 // VmRSS is in kB
		}
	}
}
return 0

}

Security Issues (1)

Severity Location Issue
🟡 Warning gateway/coprocess_test.go:391
The error from `strconv.ParseUint` is ignored. While this is in a test and the function is designed to return 0 on failure, it's a best practice to explicitly handle or acknowledge all errors to prevent unexpected behavior. If the line format of `/proc/self/status` changes, this could silently fail and return 0, causing the test to be skipped rather than indicating a parsing problem.
💡 SuggestionExplicitly handle the error from `strconv.ParseUint`. If an error occurs, you could log it for debugging purposes before returning 0. This makes the failure mode more explicit.
🔧 Suggested Fix
                val, err := strconv.ParseUint(fields[1], 10, 64)
                if err != nil {
                    // Optional: log the error for debugging in case of unexpected format.
                    // t.Logf("failed to parse VmRSS value '%s': %v", fields[1], err)
                    return 0
                }
				return val * 1024 // VmRSS is in kB
\n\n \n\n

Performance Issues (2)

Severity Location Issue
🟡 Warning gateway/coprocess_test.go:105
Calling `runtime.GC()` inside a tight loop can significantly slow down the test and may not accurately reflect real-world garbage collection behavior. The GC is invoked 100 times within this loop, which is excessive.
💡 SuggestionRemove the periodic `runtime.GC()` call from the loop. A single `runtime.GC()` call before and after the loop is sufficient to measure the heap and RSS difference, which is the goal of this test. The Go runtime will trigger GC as needed during the loop, which better simulates a real workload.
🟡 Warning gateway/coprocess_test.go:40-56
The `getRSS` function reads the entire `/proc/self/status` file into memory to find a single line. For a small proc file this is acceptable, but a more memory-efficient approach is to read the file line-by-line.
💡 SuggestionUse `bufio.Scanner` to read the file line by line. This avoids allocating memory for the entire file content and stops reading as soon as the target line (`VmRSS:`) is found, making it more efficient.
🔧 Suggested Fix
func getRSS() uint64 {
	file, err := os.Open("/proc/self/status")
	if err != nil {
		return 0
	}
	defer file.Close()
scanner := bufio.NewScanner(file)
for scanner.Scan() {
	line := scanner.Text()
	if strings.HasPrefix(line, &#34;VmRSS:&#34;) {
		fields := strings.Fields(line)
		if len(fields) &gt;= 2 {
			val, _ := strconv.ParseUint(fields[1], 10, 64)
			return val * 1024 // VmRSS is in kB
		}
	}
}
return 0

}

\n\n ### Quality Issues (3)
Severity Location Issue
🟠 Error gateway/coprocess_test.go:410-496
The test uses multiple hardcoded values for loop iterations, payload size, and assertion thresholds (e.g., 5000, 10000, 2, 20*1024*1024). These 'magic numbers' lack context, making the test's intent unclear and its assertions arbitrary. Hardcoding thresholds for memory behavior can lead to flaky tests that are sensitive to the execution environment, Go runtime version, or underlying hardware.
💡 SuggestionExtract these hardcoded values into named constants with descriptive names (e.g., `sessionPayloadSize`, `churnIterations`, `minRssGrowthBytes`, `rssToHeapGrowthFactor`). This improves readability, makes the test's logic self-documenting, and simplifies future adjustments.
🟡 Warning gateway/coprocess_test.go:399
The `TestCoProcess_MemoryFragmentation` test is resource-intensive, performing a large number of allocations in a loop (10000 iterations) and creating a large in-memory session object. This type of test can be slow and consume significant memory, potentially degrading the performance of the CI pipeline when run as part of the default test suite.
💡 SuggestionAdd a build tag (e.g., `//go:build fragmentation`) to the test file. This allows the test to be excluded from default test runs and executed explicitly when memory behavior needs to be validated, preventing it from slowing down routine CI checks.
🟡 Warning gateway/coprocess_test.go:391
In the `getRSS` helper function, the error returned by `strconv.ParseUint` is ignored. If the `VmRSS` value in `/proc/self/status` is ever non-numeric, the function will silently return 0, which could mask a parsing issue.
💡 SuggestionExplicitly check the error returned from `strconv.ParseUint`. If an error occurs, you can log it or simply return 0, but the check should be present to make the error handling explicit. For example: `val, err := strconv.ParseUint(fields[1], 10, 64); if err != nil { return 0 }`.

Powered by Visor from Probelabs

Last updated: 2026-04-21T17:11:59.783Z | Triggered by: pr_opened | Commit: 4474168

💡 TIP: You can chat with Visor using /visor ask <your question>

@github-actions
Copy link
Copy Markdown
Contributor

API Changes

no api changes detected

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.

1 participant