Cache NUGET_DISABLE_PACKAGEID_VALIDATION env var read in PackageIdValidator.Validate to eliminate per-call allocations

[nareshjo]

🤖 AI-Generated Pull Request 🤖

This pull request was generated by the VS Perf Rel AI Agent. Please review this AI-generated PR with extra care! For more information, visit our wiki. Please share feedback with TIP Insights

---

• Issue: PackageIdValidator.Validate() calls Environment.GetEnvironmentVariable("NUGET_DISABLE_PACKAGEID_VALIDATION") on every invocation.
  On .NET Framework (the Visual Studio host process), each call triggers two allocation sources: (1) a Char[] buffer from the P/Invoke return path via COMStringBuffer::ReplaceBufferInternal, and (2) a CAS (Code Access Security) permission demand via EnvironmentPermission.AddPathList, which internally allocates an EnvironmentStringExpressionSet, String[], Int32[], and Object[].

  Together this produces ~5 short-lived objects per call.

  This method is called from 8 production call sites across 7 files. In the ProcessRegistrationPage hot path, it is invoked 3× per package version via: PackageMetadataResourceV3.ProcessRegistrationPage → GetReadmeUrl / GetReportAbuseUrl / GetUri → PackageIdValidator.Validate → EnvironmentVariableWrapper.GetEnvironmentVariable → Environment.GetEnvironmentVariable.

  The hot path call tree from PerfWatson telemetry confirms that 100% of sampled allocations in this issue trace back to this single GetEnvironmentVariable call:

  ProcessRegistrationPage
  ├── PackageDetailsUriResourceV3.GetUri
  │   └── PackageIdValidator.Validate → GetEnvironmentVariable
  │     ├── Char[]
  │     └── EnvironmentStringExpressionSet
  ├── ReportAbuseResourceV3.GetReportAbuseUrl
  │   └── PackageIdValidator.Validate → GetEnvironmentVariable
  │       ├── String[], Int32[]
  │       └── EnvironmentStringExpressionSet
  └── ReadmeUriTemplateResource.GetReadmeUrl
        └── PackageIdValidator.Validate → GetEnvironmentVariable
            └── Char[]
  

• Issue type: Reduce repeated identical allocations from per-call Environment.GetEnvironmentVariable on a hot path

• Proposed fix: Cache the environment variable result in a private static readonly Lazy<bool> field that reads from EnvironmentVariableWrapper.Instance once per process. The production path (env == null) reads the cached value; the test path (env != null) reads directly from the provided IEnvironmentVariableReader to preserve testability.

  This follows the existing convention in the codebase: EnhancedHttpRetryHelper uses the identical Lazy<bool> + IEnvironmentVariableReader pattern (lines 66–72) for caching NUGET_RETRY_HTTP_429 and other environment variables.

  Allocation site	Original (per query, N = matching versions)	After fix
  Char[] (string buffer)	3N	0
  EnvironmentStringExpressionSet (CAS)	3N	0
  String[] (CAS split)	3N	0
  Int32[] (CAS split)	3N	0
  Object[] (CAS ArrayList)	3N	0
  Total objects per query	~15N	0 (1 Lazy<bool>, once per process)

• Impact: Eliminates 100% of sampled allocations in this issue's call tree. For a query returning 50 versions, this removes ~750 short-lived objects per query. The CAS-related allocations (~64% of sampled weight) are .NET Framework-specific and affect the Visual Studio host process directly.

• Safety: No behavior change — same validation logic, same exception type, same conditions. Lazy<bool> is thread-safe by default (ExecutionAndPublication). The env var is a diagnostic escape hatch with zero in-process writers (SetEnvironmentVariable is never called for this variable anywhere in the codebase). SecurityException from the underlying GetEnvironmentVariable is still handled — EnvironmentVariableWrapper catches it and returns null, which evaluates to false (validation enabled), identical to the pre-fix behavior. Existing tests pass a mock IEnvironmentVariableReader and bypass the cache entirely.

Best practices wiki
See related failure in PRISM
ADO work item