From af052a0fea83734d93f8b5bd499c3d322c0af45a Mon Sep 17 00:00:00 2001 From: Max Charlamb Date: Sun, 21 Jun 2026 16:09:14 -0400 Subject: [PATCH] Slim ThreadStressLogData and simplify GetStressMessages API Reduce ThreadStressLogData public record from 8 fields to 3: {Address, ThreadId, WriteHasWrapped}. The removed fields (NextPointer, CurrentPointer, ChunkListHead, ChunkListTail, CurrentWriteChunk) are internal traversal state that only the contract implementation needs. Change GetStressMessages to take a TargetPointer (the thread stress log address) instead of the full record. The contract re-reads the ThreadStressLog data internally from that address. This also simplifies GetStressLogMessageEnumerator in SOSDacImpl -- it no longer needs to re-enumerate all threads to find the matching one; it passes the address directly to the contract. Remove unused StressLogChunk.Prev data descriptor and field (only used at runtime for the write path, never during dump analysis). Remove unused StressLogChunkMaxSize constant. Fix pre-existing StressLogAnalyzer crash: contract version was specified as numeric 2 instead of string "c2" in the hardcoded contract descriptor JSON. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/design/datacontracts/StressLog.md | 19 ++++--------------- .../vm/datadescriptor/datadescriptor.inc | 1 - .../Contracts/IStressLog.cs | 9 ++------- .../Constants.cs | 1 - .../Contracts/StressLog.cs | 19 ++++++++----------- .../Data/StressLogChunk.cs | 1 - .../SOSDacImpl.cs | 19 ++----------------- .../tests/DumpTests/StressLogDumpTests.cs | 2 +- src/tools/StressLogAnalyzer/src/Program.cs | 2 +- .../src/StressLogAnalyzer.cs | 4 ++-- 10 files changed, 20 insertions(+), 57 deletions(-) diff --git a/docs/design/datacontracts/StressLog.md b/docs/design/datacontracts/StressLog.md index c761b8b06938be..b3695964b8e6d5 100644 --- a/docs/design/datacontracts/StressLog.md +++ b/docs/design/datacontracts/StressLog.md @@ -18,13 +18,8 @@ internal record struct StressLogData( internal record struct ThreadStressLogData( TargetPointer Address, - TargetPointer NextPointer, ulong ThreadId, - bool WriteHasWrapped, - TargetPointer CurrentPointer, - TargetPointer ChunkListHead, - TargetPointer ChunkListTail, - TargetPointer CurrentWriteChunk); + bool WriteHasWrapped); internal record struct StressMsgData( uint Facility, @@ -38,7 +33,7 @@ bool HasStressLog(); StressLogData GetStressLogData(); StressLogData GetStressLogData(TargetPointer stressLogPointer); IEnumerable GetThreadStressLogs(TargetPointer logs); -IEnumerable GetStressMessages(ThreadStressLogData threadLog); +IEnumerable GetStressMessages(TargetPointer threadStressLogAddress); bool IsPointerInStressLog(StressLogData stressLog, TargetPointer pointer); ``` @@ -67,7 +62,6 @@ Data descriptors used: | ThreadStressLog | ChunkListHead | Pointer to the head of the chunk list | | ThreadStressLog | ChunkListTail | Pointer to the tail of the chunk list | | ThreadStressLog | CurrentWriteChunk | Pointer to the chunk currently being written to | -| StressLogChunk | Prev | Pointer to the previous chunk | | StressLogChunk | Next | Pointer to the next chunk | | StressLogChunk | Buf | The data stored in the chunk | | StressLogChunk | Sig1 | First byte of the chunk signature (to ensure validity) | @@ -157,20 +151,15 @@ IEnumerable GetThreadStressLogs(TargetPointer logs) yield return new ThreadStressLogData( currentPointer, - threadStressLog.Next, threadStressLog.ThreadId, - threadStressLog.WriteHasWrapped, - threadStressLog.CurrentPtr, - threadStressLog.ChunkListHead, - threadStressLog.ChunkListTail, - threadStressLog.CurrentWriteChunk); + threadStressLog.WriteHasWrapped); currentPointer = threadStressLog.Next; } } // Return messages going in reverse chronological order, newest first. -IEnumerable GetStressMessages(ThreadStressLogData threadLog) +IEnumerable GetStressMessages(TargetPointer threadStressLogAddress) { // 1. Get the current message pointer from the log and the info about the current chunk the runtime is writing into. // Record our current read pointer as the current message pointer. diff --git a/src/coreclr/vm/datadescriptor/datadescriptor.inc b/src/coreclr/vm/datadescriptor/datadescriptor.inc index d8ad288c1e1af2..19ca1e3151e89c 100644 --- a/src/coreclr/vm/datadescriptor/datadescriptor.inc +++ b/src/coreclr/vm/datadescriptor/datadescriptor.inc @@ -641,7 +641,6 @@ CDAC_TYPE_END(ThreadStressLog) CDAC_TYPE_BEGIN(StressLogChunk) CDAC_TYPE_SIZE(sizeof(StressLogChunk)) -CDAC_TYPE_FIELD(StressLogChunk, T_POINTER, Prev, offsetof(StressLogChunk, prev)) CDAC_TYPE_FIELD(StressLogChunk, T_POINTER, Next, offsetof(StressLogChunk, next)) CDAC_TYPE_FIELD(StressLogChunk, T_ARRAY(T_UINT8), Buf, offsetof(StressLogChunk, buf)) CDAC_TYPE_FIELD(StressLogChunk, T_UINT32, Sig1, offsetof(StressLogChunk, dwSig1)) diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IStressLog.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IStressLog.cs index 4bd41735561628..5a81b03130fe2d 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IStressLog.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Abstractions/Contracts/IStressLog.cs @@ -19,13 +19,8 @@ public record struct StressLogData( public record struct ThreadStressLogData( TargetPointer Address, - TargetPointer NextPointer, ulong ThreadId, - bool WriteHasWrapped, - TargetPointer CurrentPointer, - TargetPointer ChunkListHead, - TargetPointer ChunkListTail, - TargetPointer CurrentWriteChunk); + bool WriteHasWrapped); public record struct StressMsgData( uint Facility, @@ -40,7 +35,7 @@ public interface IStressLog : IContract StressLogData GetStressLogData() => throw new NotImplementedException(); StressLogData GetStressLogData(TargetPointer stressLog) => throw new NotImplementedException(); IEnumerable GetThreadStressLogs(TargetPointer Logs) => throw new NotImplementedException(); - IEnumerable GetStressMessages(ThreadStressLogData threadLog) => throw new NotImplementedException(); + IEnumerable GetStressMessages(TargetPointer threadStressLogAddress) => throw new NotImplementedException(); bool IsPointerInStressLog(StressLogData stressLog, TargetPointer pointer) => throw new NotImplementedException(); } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs index fe5284e4460fac..c25170f26f75b2 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Constants.cs @@ -60,7 +60,6 @@ public static class Globals public const string StressLog = nameof(StressLog); public const string StressLogModuleTable = nameof(StressLogModuleTable); public const string StressLogMaxModules = nameof(StressLogMaxModules); - public const string StressLogChunkMaxSize = nameof(StressLogChunkMaxSize); public const string StressLogMaxMessageSize = nameof(StressLogMaxMessageSize); public const string StressLogChunkSize = nameof(StressLogChunkSize); public const string StressLogValidChunkSig = nameof(StressLogValidChunkSig); diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StressLog.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StressLog.cs index c5d0d7b82ce7cd..4bee76f7039124 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StressLog.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Contracts/StressLog.cs @@ -81,13 +81,8 @@ public IEnumerable GetThreadStressLogs(TargetPointer Logs) yield return new ThreadStressLogData( currentPointer, - threadStressLog.Next, threadStressLog.ThreadId, - threadStressLog.WriteHasWrapped, - threadStressLog.CurrentPtr, - threadStressLog.ChunkListHead, - threadStressLog.ChunkListTail, - threadStressLog.CurrentWriteChunk); + threadStressLog.WriteHasWrapped); currentPointer = threadStressLog.Next; } @@ -128,18 +123,20 @@ private TargetPointer GetFormatPointer(ulong formatOffset) return TargetPointer.Null; } - public IEnumerable GetStressMessages(ThreadStressLogData threadLog) + public IEnumerable GetStressMessages(TargetPointer threadStressLogAddress) { uint stressMsgHeaderSize = target.GetTypeInfo(DataType.StressMsgHeader).Size!.Value; uint pointerSize = (uint)target.PointerSize; + Data.ThreadStressLog threadLog = target.ProcessedData.GetOrAdd(threadStressLogAddress); + Data.StressLogChunk currentChunkData = target.ProcessedData.GetOrAdd(threadLog.CurrentWriteChunk); TargetPointer currentReadChunk = threadLog.CurrentWriteChunk; - TargetPointer readPointer = threadLog.CurrentPointer; + TargetPointer readPointer = threadLog.CurrentPtr; bool readHasWrapped = false; uint chunkSize = target.ReadGlobal(Constants.Globals.StressLogChunkSize); - TargetPointer currentPointer = threadLog.CurrentPointer; + TargetPointer currentPointer = threadLog.CurrentPtr; // the last written log, if it wrapped around may have partially overwritten // a previous record. Update currentPointer to reflect the last safe beginning of a record, // but currentPointer shouldn't wrap around, otherwise it'll break our assumptions about stress @@ -341,7 +338,7 @@ internal sealed class StressLog_1(Target target) : IStressLog public StressLogData GetStressLogData() => traversal.GetStressLogData(); public StressLogData GetStressLogData(TargetPointer stressLog) => traversal.GetStressLogData(stressLog); public IEnumerable GetThreadStressLogs(TargetPointer Logs) => traversal.GetThreadStressLogs(Logs); - public IEnumerable GetStressMessages(ThreadStressLogData threadLog) => traversal.GetStressMessages(threadLog); + public IEnumerable GetStressMessages(TargetPointer threadStressLogAddress) => traversal.GetStressMessages(threadStressLogAddress); public bool IsPointerInStressLog(StressLogData stressLog, TargetPointer pointer) => traversal.IsPointerInStressLog(stressLog, pointer); } @@ -354,6 +351,6 @@ internal sealed class StressLog_2(Target target) : IStressLog public StressLogData GetStressLogData() => traversal.GetStressLogData(); public StressLogData GetStressLogData(TargetPointer stressLog) => traversal.GetStressLogData(stressLog); public IEnumerable GetThreadStressLogs(TargetPointer Logs) => traversal.GetThreadStressLogs(Logs); - public IEnumerable GetStressMessages(ThreadStressLogData threadLog) => traversal.GetStressMessages(threadLog); + public IEnumerable GetStressMessages(TargetPointer threadStressLogAddress) => traversal.GetStressMessages(threadStressLogAddress); public bool IsPointerInStressLog(StressLogData stressLog, TargetPointer pointer) => traversal.IsPointerInStressLog(stressLog, pointer); } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/StressLogChunk.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/StressLogChunk.cs index afd3e426931ffd..39e2bdc232e311 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/StressLogChunk.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Contracts/Data/StressLogChunk.cs @@ -7,7 +7,6 @@ namespace Microsoft.Diagnostics.DataContractReader.Data; internal sealed partial class StressLogChunk : IData { [Field] public TargetPointer Next { get; } - [Field] public TargetPointer Prev { get; } [FieldAddress] public TargetPointer Buf { get; } diff --git a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs index 5608f9a3ede3ca..42e4f9cae1d779 100644 --- a/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs +++ b/src/native/managed/cdac/Microsoft.Diagnostics.DataContractReader.Legacy/SOSDacImpl.cs @@ -7320,23 +7320,8 @@ int ISOSDacInterface17.GetStressLogMessageEnumerator( if (!stressLogContract.HasStressLog()) return HResults.S_FALSE; - Contracts.StressLogData logData = stressLogContract.GetStressLogData(); - - // Find the matching thread - Contracts.ThreadStressLogData? matchedThread = null; - foreach (var thread in stressLogContract.GetThreadStressLogs(logData.Logs)) - { - if (thread.Address == threadStressLogAddress.ToTargetPointer(_target)) - { - matchedThread = thread; - break; - } - } - - if (matchedThread is null) - return HResults.E_INVALIDARG; - - IEnumerable messages = stressLogContract.GetStressMessages(matchedThread.Value); + TargetPointer address = threadStressLogAddress.ToTargetPointer(_target); + IEnumerable messages = stressLogContract.GetStressMessages(address); ppEnum.Interface = new SOSStressLogMsgEnum(_target, messages); } catch (System.Exception ex) diff --git a/src/native/managed/cdac/tests/DumpTests/StressLogDumpTests.cs b/src/native/managed/cdac/tests/DumpTests/StressLogDumpTests.cs index afb3b3f3ec7bd6..7d30d45afb98b4 100644 --- a/src/native/managed/cdac/tests/DumpTests/StressLogDumpTests.cs +++ b/src/native/managed/cdac/tests/DumpTests/StressLogDumpTests.cs @@ -55,7 +55,7 @@ public void CanEnumerateThreadsAndMessages(TestConfiguration config) bool foundMessages = false; foreach (ThreadStressLogData thread in threads) { - var messages = stressLog.GetStressMessages(thread).Take(10).ToList(); + var messages = stressLog.GetStressMessages(thread.Address).Take(10).ToList(); if (messages.Count > 0) { foundMessages = true; diff --git a/src/tools/StressLogAnalyzer/src/Program.cs b/src/tools/StressLogAnalyzer/src/Program.cs index b7195a218f4afb..da521ffe4a1ecc 100644 --- a/src/tools/StressLogAnalyzer/src/Program.cs +++ b/src/tools/StressLogAnalyzer/src/Program.cs @@ -585,7 +585,7 @@ private static ContractDescriptorParser.ContractDescriptor GetDescriptor(string "StressLogModuleTable": [[ 1 ], "pointer" ], }, "contracts": { - "StressLog": 2, + "StressLog": "c2", } } """"u8)!; diff --git a/src/tools/StressLogAnalyzer/src/StressLogAnalyzer.cs b/src/tools/StressLogAnalyzer/src/StressLogAnalyzer.cs index c59bab54ecd8bc..95f0a98d522263 100644 --- a/src/tools/StressLogAnalyzer/src/StressLogAnalyzer.cs +++ b/src/tools/StressLogAnalyzer/src/StressLogAnalyzer.cs @@ -32,7 +32,7 @@ internal sealed class StressLogAnalyzer( // The "end" timestamp is the timestamp of the most recent message. timeTracker.SetEndTimestamp( logs.Select( - log => outerLogContract.GetStressMessages(log).FirstOrDefault().Timestamp) + log => outerLogContract.GetStressMessages(log.Address).FirstOrDefault().Timestamp) .Max()); if (!threadFilter.HasAnyGCThreadFilter) @@ -58,7 +58,7 @@ await Parallel.ForEachAsync(logs, parallelOptions, (log, ct) => StressMsgData? earliestMessage = null; List localMessages = []; bool includeThreadMessages = true; - foreach (StressMsgData message in stressLogContract.Value!.GetStressMessages(log)) + foreach (StressMsgData message in stressLogContract.Value!.GetStressMessages(log.Address)) { numMessagesProcessed.Value++; token.ThrowIfCancellationRequested();