Skip to content

NativeMemoryProfiler exception #1235

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

Closed
WojciechNagorski opened this issue Aug 29, 2019 · 10 comments
Closed

NativeMemoryProfiler exception #1235

WojciechNagorski opened this issue Aug 29, 2019 · 10 comments
Assignees
Milestone

Comments

@WojciechNagorski
Copy link
Contributor

@dotMorten has reproduced an exception in NativeMemoryProfiler.
There is https://twitter.com/dotMorten/status/1166842655348670466

At first, he got an error:
image

Then:

Setting ArtifactsPath="./" in my manual config seems to work (null check missing there?). Now it runs for a lot longer and creates .etl files, then goes boom here:

image

@adamsitnik
Copy link
Member

@wojtpl2 are you able to repro it?

@WojciechNagorski
Copy link
Contributor Author

@adamsitnik No, I'm not.
@dotMorten Could you share some reproduction?

I have some ideas:

@WojciechNagorski
Copy link
Contributor Author

the first exception occurred in this line:

return Path.Combine(details.Config.ArtifactsPath, $"{fileName}{FileExtension}");

It is possible only if details.Config.ArtifactsPath is null. I need time to understand when this can be null.
@dotMorten If you are not able to share reproduction, please share only your configuration. I mean csproj and benchmark configuration.

@dotMorten
Copy link

dotMorten commented Aug 29, 2019

I should be able to share something once I get into the office in a bit. The second crash only occurs for some tests, and the etl file was huuuuge at that point (over 8gb). Other tests worked fine and detected native memory.
Only see two places where division by error can occur, but not sure why those values would be zero at that point.

@WojciechNagorski
Copy link
Contributor Author

I've just pushed nicer error handling. Division by zero is possible only when *.etl file doesn't contain events from BenchmarkDotNet Engine. To be honest, I don't know yet why it works that way, but I will find this bug 😄

@dotMorten
Copy link

Below is a repro for the artifacts path. I'll get back to you with the other issue, once I zero in on it (the one that fails is overly complicated so trying to dumb it down):

You'll need this package reference:

    <PackageReference Include="Esri.ArcGISRuntime.WPF">
      <Version>100.6.0</Version>
    </PackageReference>

Repro app code:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Configs;
using BenchmarkDotNet.Jobs;
using BenchmarkDotNet.Running;
using BenchmarkDotNet.Toolchains.CsProj;
using Esri.ArcGISRuntime.Geometry;
using System;
using System.Linq;

namespace BDN_Repro
{
    class Program
    {
        static void Main(string[] args)
        {
            var config = new MyConfig();
            var summary = BenchmarkRunner.Run(typeof(GeometryBuilderBenchmarks), config);
        }
    }
    public class GeometryBuilderBenchmarks
    {
        MapPointBuilder pointbuilder;
        [GlobalSetup]
        public void Setup()
        {
            pointbuilder = new MapPointBuilder(123, 456, SpatialReferences.Wgs84);
        }

        [Benchmark]
        public double MapPointBuilder_InteropOverhead_GetSetXY()
        {
            return ((pointbuilder.X += 1) + (pointbuilder.Y += 1));
        }
    }

    public class MyConfig : ManualConfig
    {
        private Job baseJob;
        public MyConfig()
        {
            baseJob = Job.MediumRun.With(CsProjClassicNetToolchain.Net48);


            Add(DefaultConfig.Instance.GetLoggers().ToArray());
            Add(DefaultConfig.Instance.GetExporters().ToArray());
            Add(DefaultConfig.Instance.GetColumnProviders().ToArray());
            Add(BenchmarkDotNet.Diagnosers.MemoryDiagnoser.Default);
            Add(new BenchmarkDotNet.Diagnostics.Windows.NativeMemoryProfiler());
            // ArtifactsPath = "./"; //This fixes the crash
        }
    }
}

@WojciechNagorski
Copy link
Contributor Author

@dotMorten Thanks for this reproduction. I've just pushed fix.

The problem was not in NativeMemoryProfiler.

When you run benchmark with your own configuration in this way:

        static void Main(string[] args)
        {
            var config = new MyConfig();
            var summary = BenchmarkRunner.Run(typeof(GeometryBuilderBenchmarks), config);
        }

Then resulted ArtifactsPath is null.

I found information in the code that in this case the default path should be used.

private static ValidationError ValidateArtifactsPath(string artifactsPath)
{
if (artifactsPath == null) // null is OK, default path will be used

I check that the null value in ArtifactsPath causes an exception for all EtwProfiler, for instance, the configuration below raises the same exception:

        public class MyConfig : ManualConfig
        {
            private Job baseJob;
            public MyConfig()
            {
                baseJob = Job.MediumRun.With(CsProjClassicNetToolchain.Net461);


                Add(DefaultConfig.Instance.GetLoggers().ToArray());
                Add(DefaultConfig.Instance.GetExporters().ToArray());
                Add(DefaultConfig.Instance.GetColumnProviders().ToArray());
                Add(BenchmarkDotNet.Diagnosers.MemoryDiagnoser.Default);
                Add(new BenchmarkDotNet.Diagnostics.Windows.EtwProfiler()); // <-- Use EtwProfiler instead of NativeMemoryProfiler
            }
        }

@dotMorten
Copy link

dotMorten commented Aug 29, 2019

@wojtpl2 I think what I'm seeing with the second crash is because the test threw an exception in a background thread.

@WojciechNagorski
Copy link
Contributor Author

@adamsitnik @dotMorten I think that PR #1237 is ready to review and this issue is ready to close.

I checked that exception in a background thread stops benchmark and changes from #1237 shows the message:
image

@adamsitnik
Copy link
Member

Fixed in #1237 a while ago, but we forgot to close the issue. So I am closing it now.

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

No branches or pull requests

3 participants