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

Setup & Cleanup versions of attribute which would run before/after each benchmark iteration #325

Closed
another-guy opened this Issue Dec 12, 2016 · 9 comments

Comments

Projects
None yet
3 participants
@another-guy

another-guy commented Dec 12, 2016

As I understand, the Setup and Cleanup attributes are invoked once per entire benchmarking session, no matter how many iterations are going to be executed.

In my use case I need some similar attribute that will instruct BenchmarkDotNet to run some code before and after each iteration. This is because I am trying to measure the creation time ("cost") of the resource-holding object which need to be disposed. Namely, I am trying to measure and compare, whether it's way more costly to create a FileStream on every file operation (Read/Write) or to keep a file stream open for a long time...

    public class FileStreamBenchmark
    {
        private FileStream _fileStream;

        [Setup] // Setup needs to run before each benchmark iteration
        public void Setup()
        {
            _fileStream = null;
        }

        [Cleanup] // Cleanup needs to run before each benchmark iteration
        public void Cleanup()
        {
            _fileStream?.Dispose();
        }

        [Benchmark]
        public void CreateFileStream()
        {
            _fileStream = File.Open("somefile.txt", FileMode.OpenOrCreate, FileAccess.ReadWrite);
            // ... some read/write code
        }
    }
@AndreyAkinshin

This comment has been minimized.

Show comment
Hide comment
@AndreyAkinshin

AndreyAkinshin Dec 12, 2016

Member

Similar issues: #270, #274

Is seems that it is a popular request. Originally, BenchmarkDotNet was designed for microbenchmarks (in this case, the Setup/Cleanup methods can easily spoil results). However, if a benchmark takes some time (e.g. more than 1ms), such feature looks valid. I'm going to implement another RunStrategy which will support such cases (+ additional analyzers which check that we could use this strategy for a given benchmark).

@another-guy, why you couldn't include the bodies of Setup and Cleanup in the CreateFileStream method?

Member

AndreyAkinshin commented Dec 12, 2016

Similar issues: #270, #274

Is seems that it is a popular request. Originally, BenchmarkDotNet was designed for microbenchmarks (in this case, the Setup/Cleanup methods can easily spoil results). However, if a benchmark takes some time (e.g. more than 1ms), such feature looks valid. I'm going to implement another RunStrategy which will support such cases (+ additional analyzers which check that we could use this strategy for a given benchmark).

@another-guy, why you couldn't include the bodies of Setup and Cleanup in the CreateFileStream method?

@another-guy

This comment has been minimized.

Show comment
Hide comment
@another-guy

another-guy Dec 12, 2016

@AndreyAkinshin I did not include the specific implementation for Setup, Benchmark, Cleanup because I don't actually have them at the moment. I was trying to write the Benchmark when I realized it seems impossible to write it the way I wanted. This is when I decided to open the issue.

Also, I keep forgetting the fact it's important for benchmark method to run for longer than one tick (or 1ms) otherwise the measurements are going to be spoiled.

another-guy commented Dec 12, 2016

@AndreyAkinshin I did not include the specific implementation for Setup, Benchmark, Cleanup because I don't actually have them at the moment. I was trying to write the Benchmark when I realized it seems impossible to write it the way I wanted. This is when I decided to open the issue.

Also, I keep forgetting the fact it's important for benchmark method to run for longer than one tick (or 1ms) otherwise the measurements are going to be spoiled.

@another-guy

This comment has been minimized.

Show comment
Hide comment
@another-guy

another-guy Dec 12, 2016

@AndreyAkinshin I see that you're going to treat this issue as an enhancement. May I ask you to consider the following point as a soft requirement?

To your point, very short benchmark method execution time (whether it's under 1ms or 1 tick -- I'm not sure) can lead to incorrect result calculation. Would it be possible generate an error when the framework detects this situation? Alternatively, it could be a warning, especially if the framework is smart enough to ignore the too short runs when calculates the avg, p0/p50/pXX, etc.

If I were the designer of the library, I'd stick to the former approach (not trying to be smart about the unreliable run measurements). This is because the former approach has simpler design; and because the latter approach loses information by ignoring some runs' and using another runs' collected data.

another-guy commented Dec 12, 2016

@AndreyAkinshin I see that you're going to treat this issue as an enhancement. May I ask you to consider the following point as a soft requirement?

To your point, very short benchmark method execution time (whether it's under 1ms or 1 tick -- I'm not sure) can lead to incorrect result calculation. Would it be possible generate an error when the framework detects this situation? Alternatively, it could be a warning, especially if the framework is smart enough to ignore the too short runs when calculates the avg, p0/p50/pXX, etc.

If I were the designer of the library, I'd stick to the former approach (not trying to be smart about the unreliable run measurements). This is because the former approach has simpler design; and because the latter approach loses information by ignoring some runs' and using another runs' collected data.

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik

adamsitnik Dec 12, 2016

Member

Namely, I am trying to measure and compare, whether it's way more costly to create a FileStream on every file operation (Read/Write) or to keep a file stream open for a long time...

please keep in mind that the number of currently opened files by a process is limited

today you can do this with sth like this:

class OpenedSingleTime
{
        private FileStream _fileStream;

        [Setup]
        public void Setup()
        {
            _fileStream = File.Open("somefile.txt", FileMode.OpenOrCreate, FileAccess.ReadWrite);
        }

        [Cleanup]
        public void Cleanup() => _fileStream?.Dispose();

        [Benchmark]
        public void CreateFileStream()
        {
            _fileStream.CallToTheMeasuredMethod();
        }
}

class OpenedEveryTime
{
        [Benchmark]
        public void CreateFileStream()
        {
            using (var fileStream = File.Open("somefile.txt", FileMode.OpenOrCreate, FileAccess.ReadWrite))
             fileStream.CallToTheMeasuredMethod();
        }
}

@AndreyAkinshin I don't like this feature because it will complicate the code a lot, and for sure will not be supported by MemoryDiagnoser

anyway maybe it could be implemented, if our Action/Func<T> would also accept the timer, and then when we are building the Engine we could do sth like:

method = timer => 
{
  setup();
  timer.Start();
  action.Invoke();
  timer.Stop();
  cleanup();
}

but I am not sure about how this could affect the code generated by JIT.

Member

adamsitnik commented Dec 12, 2016

Namely, I am trying to measure and compare, whether it's way more costly to create a FileStream on every file operation (Read/Write) or to keep a file stream open for a long time...

please keep in mind that the number of currently opened files by a process is limited

today you can do this with sth like this:

class OpenedSingleTime
{
        private FileStream _fileStream;

        [Setup]
        public void Setup()
        {
            _fileStream = File.Open("somefile.txt", FileMode.OpenOrCreate, FileAccess.ReadWrite);
        }

        [Cleanup]
        public void Cleanup() => _fileStream?.Dispose();

        [Benchmark]
        public void CreateFileStream()
        {
            _fileStream.CallToTheMeasuredMethod();
        }
}

class OpenedEveryTime
{
        [Benchmark]
        public void CreateFileStream()
        {
            using (var fileStream = File.Open("somefile.txt", FileMode.OpenOrCreate, FileAccess.ReadWrite))
             fileStream.CallToTheMeasuredMethod();
        }
}

@AndreyAkinshin I don't like this feature because it will complicate the code a lot, and for sure will not be supported by MemoryDiagnoser

anyway maybe it could be implemented, if our Action/Func<T> would also accept the timer, and then when we are building the Engine we could do sth like:

method = timer => 
{
  setup();
  timer.Start();
  action.Invoke();
  timer.Stop();
  cleanup();
}

but I am not sure about how this could affect the code generated by JIT.

@AndreyAkinshin

This comment has been minimized.

Show comment
Hide comment
@AndreyAkinshin

AndreyAkinshin Dec 12, 2016

Member

Our toolchain will not be changed, new RunStrategy will affect only some characteristics like in the ColdStart case. E.g. we can set UnrollFactor=1, InvocationCount=1, and so on. If we carefully set all the characteristics, we will get the desired Setup and Cleanup behavior for free. Of course, it doesn't make sense for nanosecond benchmark, but it looks valid for benchmark which takes seconds.
@adamsitnik, is it ok for you?

Member

AndreyAkinshin commented Dec 12, 2016

Our toolchain will not be changed, new RunStrategy will affect only some characteristics like in the ColdStart case. E.g. we can set UnrollFactor=1, InvocationCount=1, and so on. If we carefully set all the characteristics, we will get the desired Setup and Cleanup behavior for free. Of course, it doesn't make sense for nanosecond benchmark, but it looks valid for benchmark which takes seconds.
@adamsitnik, is it ok for you?

@adamsitnik

This comment has been minimized.

Show comment
Hide comment
@adamsitnik
Member

adamsitnik commented Dec 12, 2016

@AndreyAkinshin ok 👍

@another-guy

This comment has been minimized.

Show comment
Hide comment
@another-guy

another-guy Dec 13, 2016

@adamsitnik thanks for pointing out the open file limit per process. I was not intended to open multiple files in parallel. 😸

another-guy commented Dec 13, 2016

@adamsitnik thanks for pointing out the open file limit per process. I was not intended to open multiple files in parallel. 😸

@another-guy

This comment has been minimized.

Show comment
Hide comment
@another-guy

another-guy Dec 14, 2016

@adamsitnik @AndreyAkinshin

I ended up writing the code shown below. Obviously, it does not assess the performance of the single method anymore (namely, new FileStream(...)). Instead, it measures the execution time of the entire scenario.
This information is useful to me, but nevertheless I may end up need to timewatch a specific method.

    public class FileStreamAccessOptions
    {
        [Params(1, 128)]
        public int ItemsToWrite { get; set; }

        [Params(FileAccess.Write, FileAccess.ReadWrite)]
        public FileAccess Access { get; set; }

        [Params(FileMode.Append, FileMode.OpenOrCreate)]
        public FileMode Mode { get; set; }

        // TODO Add Encrypt option here.
        [Params(FileOptions.None)]
        public FileOptions Options { get; set; }

        private const FileShare FILE_SHARE = FileShare.None;
        private const int BUFFER = 4096;
        private static readonly string ObjectToSave = JsonConvert.SerializeObject(FakeDataProvider.CreateCars()[0]);

        [Benchmark]
        public void AppendOrWrite_WithReopening()
        {
            var path = Path.GetTempFileName();
            
            for (var counter = 1; counter <= ItemsToWrite; counter++)
                using (var fileStream = new FileStream(path, Mode, Access, FILE_SHARE, BUFFER, Options))
                using (var writer = new StreamWriter(fileStream))
                    writer.WriteLine(ObjectToSave);
        }

        [Benchmark]
        public void AppendOrWrite_WithoutReopening()
        {
            var path = Path.GetTempFileName();

            using (var fileStream = new FileStream(path, Mode, Access, FILE_SHARE, BUFFER, Options))
            using (var writer = new StreamWriter(fileStream))
                for (var counter = 1; counter <= ItemsToWrite; counter++)
                    writer.WriteLine(ObjectToSave);
        }
    }

another-guy commented Dec 14, 2016

@adamsitnik @AndreyAkinshin

I ended up writing the code shown below. Obviously, it does not assess the performance of the single method anymore (namely, new FileStream(...)). Instead, it measures the execution time of the entire scenario.
This information is useful to me, but nevertheless I may end up need to timewatch a specific method.

    public class FileStreamAccessOptions
    {
        [Params(1, 128)]
        public int ItemsToWrite { get; set; }

        [Params(FileAccess.Write, FileAccess.ReadWrite)]
        public FileAccess Access { get; set; }

        [Params(FileMode.Append, FileMode.OpenOrCreate)]
        public FileMode Mode { get; set; }

        // TODO Add Encrypt option here.
        [Params(FileOptions.None)]
        public FileOptions Options { get; set; }

        private const FileShare FILE_SHARE = FileShare.None;
        private const int BUFFER = 4096;
        private static readonly string ObjectToSave = JsonConvert.SerializeObject(FakeDataProvider.CreateCars()[0]);

        [Benchmark]
        public void AppendOrWrite_WithReopening()
        {
            var path = Path.GetTempFileName();
            
            for (var counter = 1; counter <= ItemsToWrite; counter++)
                using (var fileStream = new FileStream(path, Mode, Access, FILE_SHARE, BUFFER, Options))
                using (var writer = new StreamWriter(fileStream))
                    writer.WriteLine(ObjectToSave);
        }

        [Benchmark]
        public void AppendOrWrite_WithoutReopening()
        {
            var path = Path.GetTempFileName();

            using (var fileStream = new FileStream(path, Mode, Access, FILE_SHARE, BUFFER, Options))
            using (var writer = new StreamWriter(fileStream))
                for (var counter = 1; counter <= ItemsToWrite; counter++)
                    writer.WriteLine(ObjectToSave);
        }
    }
@AndreyAkinshin

This comment has been minimized.

Show comment
Hide comment
@AndreyAkinshin

AndreyAkinshin May 30, 2017

Member

Implemented, now we have [GlobalSetup], [GlobalCleanup], [IterationSetup], [IterationCleanup].

Member

AndreyAkinshin commented May 30, 2017

Implemented, now we have [GlobalSetup], [GlobalCleanup], [IterationSetup], [IterationCleanup].

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment