Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dotnet-trace (.netrace) support #95

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from

Conversation

ivberg
Copy link
Contributor

@ivberg ivberg commented May 23, 2022

No description provided.

{
public string[] CallStack { get; set; }
public TraceModuleFile Module { get; set; }
public string FullMethodName { get; set; }
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe make readonly?

// The DataSourceInfo is used to tell analzyer the time range of the data(if applicable) and any other relevant data for rendering / synchronizing.

return this.dataSourceInfo;

Copy link
Contributor

Choose a reason for hiding this comment

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

nit: extra line


foreach (var path in this.filePaths)
{
var traceStartTime = DateTime.UtcNow.Date;
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed?

// EventPipeEventSource doesn't expose the callstacks - https://github.com/Microsoft/perfview/blob/main/src/TraceEvent/EventPipe/EventPipeFormat.md
// But currently it's SessionDuration, SessionStartTime are correct
// Can remove when when this is released - https://github.com/microsoft/perfview/pull/1635
var dotnetFileStream = new FileStream(path, FileMode.Open, FileAccess.Read, FileShare.Read);
Copy link
Contributor

Choose a reason for hiding this comment

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

add using

var baseProjection = Projection.Index(ThreadSamplingEvents);


tableGenerator.AddColumn(countColumn, baseProjection.Compose(x => 1)); // 1 sample
Copy link
Contributor

Choose a reason for hiding this comment

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

Projection.Constant(1)

tableGenerator.AddColumn(moduleColumn, baseProjection.Compose(x => x.Module?.Name));
tableGenerator.AddColumn(functionColumn, baseProjection.Compose(x => x.FullMethodName));
tableGenerator.AddHierarchicalColumn(callStackColumn, baseProjection.Compose(x => x.CallStack), new ArrayAccessProvider<string>());
var timeStampProjection = Projection.CreateUsingFuncAdaptor((i) => ThreadSamplingEvents[i].Timestamp);
Copy link
Contributor

Choose a reason for hiding this comment

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

Could compose work here?
baseProjection.Compose(x => x.Timestamp)

…g trace session info. Include some updated docs and developer info
}
catch (Exception e)
{
if (e.Message != ReadPastEndOfStreamExceptionMessage || !traceEventProcessor.HasTraceData())
Copy link
Member

Choose a reason for hiding this comment

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

If a failure occurs when calling CreateFromEventPipeDataFile, you might consider attempting to re-process the file with ContinueOnError==true. If this succeed,s you should tell users that there was an error, and you stopped at the point in the file where the error occurred, so the file might not contain all of the expected data.

{
internal class EventStat
{
public string ProviderName { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

TraceLog.Stats can provide this information for each type of event.

}

var firstTraceProcessorEventsParsed = TraceEventProcessor.First().Value; // First Log
var gcStartEvents = firstTraceProcessorEventsParsed.GenericEvents.Where(f => f.ProviderName == "Microsoft-Windows-DotNETRuntime" &&
Copy link
Member

Choose a reason for hiding this comment

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

Pretty cool! I would recommend sourcing this information from the state machine that knows how to build up the list of GCs instead of using GC/Start and GC/Stop events. This will provide you with more information if it's available, but will also handle cases where you might have nested GC/Start/Stop events (e.g. in the case of background GCs where a foreground gen0 is triggered). You can see where this gets created in PerfView here: https://github.com/microsoft/perfview/blob/main/src/PerfView/PerfViewData.cs#L3532

@brianrob
Copy link
Member

brianrob commented Jun 9, 2022

@ivberg, this is pretty great!

@MESINDUPA
Copy link

follow

@ivberg
Copy link
Contributor Author

ivberg commented Dec 6, 2022

follow

Do you have a comment or would use this feature @MESINDUPA ?

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.

None yet

4 participants