Extract ManifestBuilder and EventListener#124323
Conversation
github-actions
bot
added
the
needs-area-label
There was a problem hiding this comment.
Pull request overview
Refactors System.Diagnostics.Tracing implementation by extracting large helper types out of EventSource.cs into dedicated files, reducing the size of EventSource.cs while preserving behavior.
Changes:
- Moved
ManifestBuilderimplementation intoManifestBuilder.cs. - Moved
EventListenerimplementation intoEventListener.cs. - Updated
System.Private.CoreLib.Shared.projitemsto compile the new files.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/ManifestBuilder.cs | New file containing the extracted ManifestBuilder implementation. |
| src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventSource.cs | Removed in-file ManifestBuilder/EventListener definitions and kept remaining tracing types. |
| src/libraries/System.Private.CoreLib/src/System/Diagnostics/Tracing/EventListener.cs | New file containing the extracted EventListener implementation. |
| src/libraries/System.Private.CoreLib/src/System.Private.CoreLib.Shared.projitems | Adds the two new .cs files to the build. |
| private void WriteNameAndMessageAttribs(StringBuilder? stringBuilder, string elementName, string name) | ||
| { | ||
| stringBuilder?.Append(" name=\"").Append(name).Append('"'); | ||
| WriteMessageAttrib(sb, elementName, name, name); |
There was a problem hiding this comment.
WriteNameAndMessageAttribs appends the name= attribute to its stringBuilder parameter but emits the message= attribute via the sb field (not the parameter). This makes the helper internally inconsistent and easy to misuse if it’s ever called with a different builder. Consider passing stringBuilder through to WriteMessageAttrib (or removing the parameter if the intent is to always write to sb).
| WriteMessageAttrib(sb, elementName, name, name); | |
| WriteMessageAttrib(stringBuilder, elementName, name, name); |
| // If channelKeyworkds is present, and has keywords bits in the ValidPredefinedChannelKeywords then it is | ||
| // assumed that the keyword for that channel should be that bit. | ||
| // otherwise we allocate a channel bit for the channel. | ||
| // explicit channel bits are only used by WCF to mimic an existing manifest, | ||
| // so we don't dont do error checking. | ||
| public ulong GetChannelKeyword(EventChannel channel, ulong channelKeyword = 0) |
There was a problem hiding this comment.
Spelling/grammar in this comment block: “channelKeyworkds” should be “channelKeywords”, and “we don't dont do error checking” should be “we don't do error checking”.
| EventPipeEventDispatcher.Instance.SendCommand(this, EventCommand.Update, false, EventLevel.LogAlways, (EventKeywords)0); | ||
| #endif // FEATURE_PERFTRACING | ||
| } | ||
| // We don't expose a Dispose(bool), because the contract is that you don't have any non-syncronous |
There was a problem hiding this comment.
Spelling in comment: “non-syncronous” should be “non-synchronous”.
| // We don't expose a Dispose(bool), because the contract is that you don't have any non-syncronous | |
| // We don't expose a Dispose(bool), because the contract is that you don't have any non-synchronous |
2 participants
This reduces the size of the EventSource file, which I find helps reduce token usage