Skip to content

Optimize Linux env var handling: avoid allocations when environment not accessed#43

Merged
adamsitnik merged 10 commits intomainfrom
copilot/optimize-linux-env-vars-handling
Dec 19, 2025
Merged

Optimize Linux env var handling: avoid allocations when environment not accessed#43
adamsitnik merged 10 commits intomainfrom
copilot/optimize-linux-env-vars-handling

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Dec 19, 2025

Optimization Plan: Linux Environment Variables

  • Update native C code to handle NULL envp parameter by using extern environ
  • Modify SafeChildProcessHandle.Linux.cs to check HasEnvironmentBeenAccessed
  • Avoid calling GetEnvironmentVariables when env vars haven't been touched
  • Pass null for envp when environment wasn't accessed
  • Add test to verify child inherits environment correctly
  • Run benchmarks before changes to establish baseline
  • Run benchmarks after changes to measure improvement
  • Verify all existing tests still pass
  • Address all PR review feedback

Benchmark Results

Before Optimization

| Method   | Mean     | Allocated |
|--------- |---------:|----------:|
| New      | 179.9 ms | 102.48 KB |
| NewAsync | 213.9 ms |  104.5 KB |

After Optimization

| Method   | Mean     | Allocated | Improvement    |
|--------- |---------:|----------:|---------------:|
| New      | 178.4 ms |   0.57 KB | 156x less mem  |
| NewAsync | 213.6 ms |   2.59 KB |  34x less mem  |

Summary

Successfully optimized Linux environment variable handling by:

  • Checking HasEnvironmentBeenAccessed before allocating native memory
  • Passing null to native layer when environment not touched
  • Using extern char **environ directly in C code when envp is NULL

Results:

  • 156x less memory allocation for synchronous execution (102KB → 0.57KB)
  • 34x less memory allocation for async execution (104KB → 2.6KB)
  • No performance regression - actually slightly faster
  • All 13 SafeChildProcessHandleTests pass

All PR Review Feedback Addressed:

  • Using modern C# is not null syntax
  • Simplified FreeArray call without redundant null check
  • Added detailed comment explaining extern char **environ
  • Test simplified with Assert.Single for both assertions
  • Test works consistently on all OS
  • Removed _codeql_detected_source_root file
Original prompt

This section details on the original issue you should resolve

<issue_title>Optimize Linux implementation when no env vars were touched by the user</issue_title>
<issue_description>The Linux implementation allocates an array of native utf8 strings that are null terminated when mapping managed env vars to native env vars:

UnixHelpers.AllocNullTerminatedArray(envp, ref envpPtr);

We should detect the case when ProcessStartOptions.HasEnvironmentBeenAccessed returns false and do the following:

  • don't access ProcessStartOptions.Environment to avoid allocations
  • don't allocate the native memory here:
    UnixHelpers.AllocNullTerminatedArray(envp, ref envpPtr);
  • just pass null to the native layer and make it pass environ directly to execve.

By doing that we are going to avoid needles utf8->unicode->utf8 converstion for all the env vars in case where user has not modified the env vars and the process should simply derive the env vars from parent.

</issue_description>

<agent_instructions>Run benchmarki before and after and share the results</agent_instructions>

Comments on the Issue (you are @copilot in this section)


💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

Copilot AI and others added 3 commits December 19, 2025 18:49
- Check HasEnvironmentBeenAccessed before allocating env vars
- Pass null to native layer when environment not accessed
- Use extern environ in C code when envp is NULL
- Update test to access Environment property when needed
- Add new test for environment inheritance without access

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
- Clarify comment about .NET environment variable behavior
- Extract env variable to improve C code readability

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
…ot accessed

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copilot AI changed the title [WIP] Optimize Linux implementation for unaccessed environment vars Optimize Linux env var handling: avoid allocations when environment not accessed Dec 19, 2025
Copilot AI requested a review from adamsitnik December 19, 2025 18:56
Copy link
Copy Markdown
Owner

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

@copilot please address my feedback

Comment thread Library/SafeChildProcessHandle.Linux.cs Outdated
Comment thread Library/SafeChildProcessHandle.Linux.cs Outdated
Comment thread Library/native/process_spawn.c
Comment thread Tests/SafeChildProcessHandleTests.cs Outdated
- Use 'is not null' syntax instead of '!= null'
- Simplify FreeArray call by passing null-coalesced length
- Add detailed comment explaining extern char **environ
- Improve test to set unique env var and verify both cases

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Dec 19, 2025

@copilot please address my feedback

All feedback addressed in commit 020aedd:

  • Using is not null syntax
  • Simplified FreeArray call
  • Added detailed comment about extern environ
  • Enhanced test with unique env var verification

Copy link
Copy Markdown
Owner

@adamsitnik adamsitnik left a comment

Choose a reason for hiding this comment

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

@copilot address my feedback

Comment thread Tests/SafeChildProcessHandleTests.cs Outdated
Comment thread Tests/SafeChildProcessHandleTests.cs Outdated
Comment thread Tests/SafeChildProcessHandleTests.cs Outdated
Comment thread Tests/SafeChildProcessHandleTests.cs Outdated
Comment thread Tests/SafeChildProcessHandleTests.cs Outdated
Comment thread _codeql_detected_source_root Outdated
- Remove _codeql_detected_source_root file
- Simplify test to use Assert.Single with manual loop
- Ensure test works on all OS by accessing Environment
- Remove OS-specific conditional assertions
- Use Windows commands for options2 as well

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Dec 19, 2025

@copilot address my feedback

All feedback addressed in commit f56368e:

  • Using Assert.Single with manual iteration
  • Test works consistently on all OS
  • Removed conditional directives
  • Added Windows options for second test
  • Removed _codeql_detected_source_root file

Comment thread Tests/SafeChildProcessHandleTests.cs Outdated
Use Assert.Single directly with IEnumerable cast instead of manual loop

Co-authored-by: adamsitnik <6011991+adamsitnik@users.noreply.github.com>
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Dec 19, 2025

Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details.

Warning

Firewall rules blocked me from connecting to one or more addresses (expand for details)

I tried to connect to the following addresses, but was blocked by firewall rules:

  • http://crl3.digicert.com:443/DigiCertCSRSA4096RootG5.crl
    • Triggering command: /usr/bin/dotnet dotnet build -c Release (http block)
  • http://crl3.digicert.com:443/NETFoundationProjectsCodeSigningCA2.crl
    • Triggering command: /usr/bin/dotnet dotnet build -c Release (http block)
  • http://crl3.digicert.com:443/sha2-assured-ts.crl
    • Triggering command: /usr/bin/dotnet dotnet build -c Release (http block)

If you need me to access, download, or install something from one of these locations, you can either:

@adamsitnik adamsitnik force-pushed the copilot/optimize-linux-env-vars-handling branch from 218a7bc to d8a0a3b Compare December 19, 2025 23:27
@adamsitnik adamsitnik marked this pull request as ready for review December 19, 2025 23:39
@adamsitnik adamsitnik merged commit 33c56b1 into main Dec 19, 2025
3 checks passed
@adamsitnik adamsitnik deleted the copilot/optimize-linux-env-vars-handling branch December 19, 2025 23:41
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.

Optimize Linux implementation when no env vars were touched by the user

2 participants