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

Q: Include hardware counters in XML output #2233

Closed
zvrba opened this issue Dec 20, 2022 · 7 comments · Fixed by #2458
Closed

Q: Include hardware counters in XML output #2233

zvrba opened this issue Dec 20, 2022 · 7 comments · Fixed by #2458

Comments

@zvrba
Copy link

zvrba commented Dec 20, 2022

Hi. I've lost a couple of hours now trying to coax BenchmarkDotNet to output hardware counters in a XML or JSON file. Default job configuration outputs them in the console report and CSV file, but not matter what I do (even following CSV export example), I run into the following troubles:

  • The HW counters are missing in the XML or JSON file, but present in the CSV output
  • Configuring CSV exporter to write machine-readable output (as below) does not work; i get units in values columns

HW counters DO get shown in the console summary.

Any suggestions?

Sample code:

// In main: var summaries = BenchmarkSwitcher.FromAssembly(typeof(Program).Assembly).Run(config: new CustomConfig());

internal class CustomConfig : ManualConfig
{
    public CustomConfig() {
        AddJob(Job.Default);
        AddLogger(BenchmarkDotNet.Loggers.ConsoleLogger.Default);
        AddColumn(BenchmarkDotNet.Columns.)

        AddExporter(new CsvExporter(
            CsvSeparator.CurrentCulture,
            new SummaryStyle(
                cultureInfo: System.Globalization.CultureInfo.InvariantCulture,
                printUnitsInHeader: true,
                printUnitsInContent: false,
                timeUnit: Perfolizer.Horology.TimeUnit.Millisecond,
                sizeUnit: BenchmarkDotNet.Columns.SizeUnit.KB)));
        AddHardwareCounters(HardwareCounter.TotalIssues, HardwareCounter.CacheMisses, HardwareCounter.BranchInstructions, HardwareCounter.BranchMispredictions);
    }
}
@adamsitnik
Copy link
Member

Hi!

We are most likely just missing this information. Please send a PR that fixes it, you should be able to achieve that by modifying the JSON exporter here:

var benchmark = new Dictionary<string, object>
{
// We don't need Benchmark.ShortInfo, that info is available via Benchmark.Parameters below
{ "DisplayInfo", report.BenchmarkCase.DisplayInfo },
{ "Namespace", report.BenchmarkCase.Descriptor.Type.Namespace },
{ "Type", FullNameProvider.GetTypeName(report.BenchmarkCase.Descriptor.Type) },
{ "Method", report.BenchmarkCase.Descriptor.WorkloadMethod.Name },
{ "MethodTitle", report.BenchmarkCase.Descriptor.WorkloadMethodDisplayInfo },
{ "Parameters", report.BenchmarkCase.Parameters.PrintInfo },
{
"FullName", FullNameProvider.GetBenchmarkName(report.BenchmarkCase)
}, // do NOT remove this property, it is used for xunit-performance migration
// Hardware Intrinsics can be disabled using env vars, that is why they might be different per benchmark and are not exported as part of HostEnvironmentInfo
{ "HardwareIntrinsics", report.GetHardwareIntrinsicsInfo() ?? "" },
// { "Properties", r.Benchmark.Job.ToSet().ToDictionary(p => p.Name, p => p.Value) }, // TODO
{ "Statistics", report.ResultStatistics }
};

and DTOs used by the XML exporter here:

internal class BenchmarkReportDto
{
public string DisplayInfo => report.BenchmarkCase.DisplayInfo;
public string Namespace => report.BenchmarkCase.Descriptor.Type.Namespace;
public string Type => report.BenchmarkCase.Descriptor.Type.Name;
public string Method => report.BenchmarkCase.Descriptor.WorkloadMethod.Name;
public string MethodTitle => report.BenchmarkCase.Descriptor.WorkloadMethodDisplayInfo;
public string Parameters => report.BenchmarkCase.Parameters.PrintInfo;
public Statistics Statistics => report.ResultStatistics;

We have tests that verify the output of the exported files, you will need to update them as well.

cd .\tests\BenchmarkDotNet.Tests\
dotnet test -c Release -f net7.0

@nazulg
Copy link
Contributor

nazulg commented Oct 30, 2023

Hi @adamsitnik !

I can take this issue and fix it.

I noted the following with the initial request,

  1. The default JsonExporter is already writing the Hardware Counters. I tested it locally with the latest code (from master) and I also noted it in the source code file here:

if (report.Metrics.Any())
{
benchmark.Add("Metrics", report.Metrics.Values);
}

  1. The default XmlExporter is missing the Hardware Counters in the default configuration. I will add those in the SummaryDto.cs file as specified by @adamsitnik .

  2. The sample code from the initial user works fine, but it was missing the MetricColumn(s) in the AddColumn in order to output the counters in the configuration. I noticed that StatisticColumn has an AllStatistics field that can be used in the Config, but the MetricColumn seems to be missing something similar. So my idea is to create something similar to StatisticColumn.AllStatistics but for the MetricColumn (example: Metric.CacheMisses, etc.), this could be in a different PR.

@adamsitnik
Copy link
Member

Hi @nazulg

I can take this issue and fix it.

Awesome!

I will add those in the SummaryDto.cs

👍

it was missing the MetricColumn(s) in the AddColumn in order to output the counters in the configuration

Would it be possible to always add the required columns when AddHardwareCounters is called?

@nazulg
Copy link
Contributor

nazulg commented Oct 31, 2023

Hi @adamsitnik!,

Great! Yes, that will be better to add the required columns based on the AddHardwareCounters call, I will get this change too.

Thank you.

@nazulg
Copy link
Contributor

nazulg commented Nov 1, 2023

Hi @adamsitnik ,

I am trying to push a PR but getting a 403 permissions error, do I need permissions to the repository to send a PR?

"remote: Permission to dotnet/BenchmarkDotNet.git denied to nazulg"

Thank you.

@timcassell
Copy link
Collaborator

@nazulg You need to fork the repository, make a branch in your fork and apply the changes, then open the PR with that branch from your fork. There's a button at the top of the main page to easily fork the repo.

image

nazulg added a commit to nazulg/BenchmarkDotNet that referenced this issue Nov 1, 2023
…lumns for output based on Hardware Counters when using ManualConfig. Fix dotnet#2233
@nazulg
Copy link
Contributor

nazulg commented Nov 1, 2023

Thank you @timcassell ! I created a draft PR,

I included verified text files since they were needed in my local to pass the test, but not sure if they should be included?

Thank you.

timcassell pushed a commit that referenced this issue Nov 2, 2023
* Adds Metrics Columns to Benchmark Report for XmlExporter, and adds columns for output based on Hardware Counters when using ManualConfig. Fix #2233

* Removed changes to ManualConfig that were not needed and introduced in a past commit of this PR.
@timcassell timcassell added this to the v0.13.11 milestone Nov 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment