Skip to content

feat: Add RAM Frequency and Type to Environment Summary (#3041) - #3281

Open
cloudsealed wants to merge 16 commits into
dotnet:masterfrom
cloudsealed:feature/issue-3041
Open

cloudsealed wants to merge 16 commits into
dotnet:masterfrom
cloudsealed:feature/issue-3041

Conversation

@cloudsealed

Copy link
Copy Markdown
Contributor

Fixes #3041

Description

This PR implements the community-requested feature to read and display RAM frequency (MHz) and memory type (e.g., DDR4) in the environment summary.

It updates PhysicalMemoryInfo to parse WMI (Win32_PhysicalMemory) on Windows and /usr/sbin/dmidecode on Linux.

Testing

  • Added PhysicalMemoryInfoTests to validate memory info parsing without allocations.
  • dotnet test passed 100% on net10.0 and net8.0.
  • Zero-allocation logic using fast type parsing.

Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs Outdated
Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs Outdated
Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs Outdated
Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs Outdated
Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs Outdated

@timcassell timcassell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Can you implement it for MacOS also?

return null;
}

private static (long? FrequencyMHz, string? MemoryType) GetLinuxOrWindowsMemoryDetails()

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It looks like this should be 2 separate methods.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Please address

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You did not address. GetLinuxOrWindowsMemoryDetails should be split to GetLinuxMemoryDetails and GetWindowsMemoryDetails.

Implement GetMacMemoryDetails() using system_profiler SPMemoryDataType to extract
memory speed and type information on macOS, matching Windows and Linux implementations.
Addresses feedback from @timcassell on PR dotnet#3281.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@cloudsealed

Copy link
Copy Markdown
Contributor Author

Hi @timcassell, I implemented macOS support as requested. The new GetMacMemoryDetails() method uses system_profiler SPMemoryDataType to extract memory speed and type info, matching the Windows and Linux implementations.

The implementation:

  • Queries system_profiler for DDR/LPDDR type detection
  • Extracts MHz frequency from the 'Speed:' field
  • Falls back gracefully if info unavailable
  • Compiled without warnings/errors and passed unit tests

Ready for review. Let me know if you'd like any adjustments.

…cs()

Extract frequency and memory type detection into a dedicated method to match
the architectural pattern used by Windows (GetWindowsSmbiosMemoryDetails) and
Linux (GetLinuxOrWindowsMemoryDetails). Addresses feedback from @timcassell
that implementation should use separate methods.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@cloudsealed

Copy link
Copy Markdown
Contributor Author

Updated to address your feedback about separate methods. Refactored the macOS implementation:

✅ Created dedicated GetMacMemorySpecs() method for frequency and type extraction
✅ Matches the architectural pattern of GetWindowsSmbiosMemoryDetails() and GetLinuxOrWindowsMemoryDetails()
✅ Single system_profiler call for efficiency
✅ Graceful fallback on error

Ready for review. Commit: 1eb5367

Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs
Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs Outdated
Use ProcessHelper.RunAndReadOutput instead of manual ProcessStartInfo
calls in GetMacMemory() and GetMacMemorySpecs() to avoid potential
deadlock when system_profiler output exceeds the stdout buffer size.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs Outdated
Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs Outdated
@filzrev

filzrev commented Oct 10, 2026

Copy link
Copy Markdown
Contributor

I've confirmed latest CI logs.
https://github.com/dotnet/BenchmarkDotNet/actions/runs/38045693391

Though, it seems failed to gets detailed memory information and fallback path is used.
(Additionally, On current PR, there is not GetLinuxMemory don't gets frequency information)

And macos(arm64) CI. following information is printed.

Memory: 7 GB (U), 0.55 GB Available

cloudsealed and others added 2 commits October 10, 2026 16:58
- Convert MHz to MT/s (Megatransfers per second) for DDR memory types
- Update FrequencyMHz property to FrequencyMts
- Apply 2x multiplier: MT/s = MHz × 2 for DDR memory
- Remove "Unspecified" filter from Linux memory type detection
- Update unit label in output from MHz to MT/s

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
Use non-whitespace pattern (\S+) instead of uppercase-only pattern
to properly capture memory type strings like UDIMM, LRDIMM, etc.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
@timcassell

Copy link
Copy Markdown
Collaborator

It wasn't explicitly requested in the issue, but it would be nice to include memory timings also, if the info is available.

- Add CasLatency property to PhysicalMemoryInfo
- Extract CAS latency from SMBIOS Type 17 offset 0x1C on Windows
- Display in format: 8GB (DDR4-6400MT/s-CAS16)
- Update unit tests to verify CAS latency property

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
freq = speedMhz * 2; // Convert MHz to MT/s
}

var typeMatch = Regex.Match(output, @"Type:\s+(\S+)");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On macos(arm64) CI environment.
It looks like not able to gets memory details information.

Memory:

      Memory: 7 GB
      Type: Unknown
      Manufacturer: Unknown

And following information is printed.

Memory: 7 GB (Unknown), 0.56 GB Available

I though it's better to remove (Unknown) because it's confusing what's Unknow means.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

On macos(x64).

It looks like memory type information can't get as expected.
And printed as following.

Memory: 14 GB (RAM-3200MT/s), 0.6 GB Available

{
try
{
string? output = ProcessHelper.RunAndReadOutput("system_profiler", "SPMemoryDataType");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I've not have macos environment thought.
It looks like system_profiler SPMemoryDataType command results can be localized.
So it might be better to use -json option. and parse results with System.Text.Json

Replace regex-based parsing of localized system_profiler text with JSON
output. This eliminates issues with different locales and handles varying
output formats on different macOS versions.

Changes:
- Use system_profiler -json SPMemoryDataType instead of text parsing
- Parse JSON with System.Text.Json for robustness
- Filter out "Unknown" memory types from output
- Extract speed and type from either direct properties or _properties array
- Gracefully handle missing or malformed output

Addresses feedback on regex fragility and locale-specific output variations.
Comment thread src/BenchmarkDotNet/Environments/PhysicalMemoryInfo.cs Outdated
byte cas = buffer[offset + 0x1C];
if (cas > 0 && cas < 100) // Reasonable CAS latency range
{
casLatency = cas;

@timcassell timcassell Oct 11, 2026 •

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

At least in CI, this value is not included (I haven't tested locally). Unsure if the cas < 100 guard is really necessary. And cas > 0 is redundant (byte is [0, 255]).

Display 'GB Total' instead of just 'GB' to clarify total memory vs available memory.
Output now shows: '16 GB Total (DDR4-6400MT/s-CAS16), 8 GB Available'

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SMBIOS specification define this speed unit as MT/s
So this logic for DDR memory should be removed.

SMBIOS specification defines memory speed at offset 0x15 as MT/s,
not MHz. Remove the DDR multiplication logic (speedMhz * 2) since
the value is already in the correct unit.

Also simplify CAS latency validation by removing the unnecessary
cas < 100 guard, keeping only cas > 0.

Co-Authored-By: Claude Haiku 4.5 <noreply@anthropic.com>
}
}

// CAS Latency at offset 0x1C (BYTE)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

As far as I've confirmed SMBIOS Specification.
https://www.dmtf.org/sites/default/files/standards/documents/DSP0134_3.10.0.pdf

0x1C is defined as Extended Size field (That is used for represent size over 32GB)
And it looks like there is no field to get CAS Latency

This branch has not been deployed

No deployments
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.

Feature: Add RAM Frequency and Type to Environment Summary

3 participants