Dynamic OperationsPerInvoke

[summivox]

I load a list of strings externally as data for my benchmarks. My goal is to measure average time for me to process one string. Of course the data is loaded before benchmarking, but since it happens at runtime, I cannot supply OperationsPerInvoke attribute param to BenchmarkAttribute, which requires compile-time constant. Is there a way around?

[redknightlois]

@AndreyAkinshin Any idea when this is gonna be implemented? Cause we really need it. If you didnt started already I can tell @jbayardo to implement it and send the PR.

[AndreyAkinshin]

Hi @redknightlois. It seems easy to implement but I'm not sure about API. Do you have any ideas how it should look? Also, I want to add an ability to specify dynamic Params (a method which returns values for Params field/property instead of a constant array). It would be cool to use the same approach for the both cases.

[summivox]

For OperationsPerInvoke: My preference is for the benchmark method to return an int showing how many iterations it actually run.

[redknightlois]

I use returning values to avoid dead code elimination issues, so the result approach could not be used generically. Having an Attribute that you can bind to a property could be more enough. Also providing a delegate to the Job configuration could work too.

[summivox]

@redknightlois while I also used return value as an attempt to prevent dead code elimination, I feel there could be a more explicit way (e.g. Benchmark.Sink(value)).

Using return value has the following advantages: it's still specified in the benchmark method, does not require further reflection, can be easily extended to async (Task<int>).

[redknightlois]

If it can be done, it is actually a very good idea to have a Benchmark.Sink(value) or Benchmark.PreventElimination(value)

Probably it could be done with something like this:

[MethodImpl(MethodImplOptions.NoInlining | MethodImplOptions.NoOptimization | MethodImplOptions.PreserveSig)]
public void PreventElimination<T>(T value)
{ }

We should look into the assembler to be sure, but sounds like a viable thing.

[summivox]

@AndreyAkinshin : Just realized that in order for this to be backwards-compatible, the benchmark method should still be tagged with an attribute (e.g. ReturnOperationsPerInvoke).

Fiddling with code now.

[AndreyAkinshin]

I think, we shouldn't use the returned value as OperationPerInvoke. It looks like a hack, it force you to design the method body in a special way, it can not be generalized for other benchmark attributes.

it is actually a very good idea to have a Benchmark.Sink(value) or Benchmark.PreventElimination(value)

@redknightlois, we will have such method in v0.10.0, I'm working on it right now (it's the last thing in the engine which I want to finish for this release). Unfortunately, it's not easy to implement it right way. Your PreventElimination<T> will not work well and could spoil some benchmark results. It will be possible to use my new Sink method inside a benchmark, but I'm not sure that it's a good idea in general case: it's still easy to design a wrong microbenchmark.

So, I suggest to continue to use returning values to avoid dead code elimination, and come up with another way to specify OperationPerInvoke.

[dazinator]

Any further news on this?

[AndreyAkinshin]

@dazinator, I definitely want to implement it in the nearest future, but I still don't know how the API should look like.

[dazinator]

How about yielding an IEnumerable<>, where each item yielded would represent one operation and therefore the total operations would be something doing a Count() on the other end.. I just raised an issue showing that kind of pattern. #323
If people are already legitimately retuning IEnumerables that we don't want to interpret as operations then can always strong type the T so for example you'd have to yield IEnumerable<OperationResult> or something..

[ig-sinicyn]

@AndreyAkinshin
I've investigated this some time ago. I'd prefer to have this as an attribute applied to the benchmark class, not to the specific method.

[dazinator]

If the benchmark method yielded an OperationResult, that wouldn't need any attributes either.. ?

[AndreyAkinshin]

@adamsitnik what do you think, is it possible to easily solve this issue with the same approach which we are using for dynamic values in params?

[adamsitnik]

@AndreyAkinshin I am not sure if I understand the idea. When the user writes a benchmark, the number of OperationsPerInvoke is not know, but discovered at run time?

Is the value always the same for all iterations?

[AndreyAkinshin]

The value should be the same for all iteration, otherwise API will be too complicated and it will be very hard to provide good accuracy in the general case. I think we should support cases when OperationsPerInvoke is discovered in GlobalSetup. Thus, this value can depend on external data (e.g. files or database) or on param values.

[adamsitnik]

I can see that it would be nice to allow the users to "return" something from the benchmarks. Some people want to check the size of compressed file (to add the size to the perf comparison), some want to add something else (like information about vectorization support). I will try to come up with some universal solution which is not going to affect our accuracy

[AndreyAkinshin]

I can see that it would be nice to allow the users to "return" something from the benchmarks.

I am against this idea of using return for it. It will produce a lot of troubles (like how to keep these values and don't spoil accuracy for microbenchmarks or what should we in case if a method returns a new value each time). I understand the people want to provide some additional values for the summary table, but they should do it without return. Probably it makes sense to add an API which can be used in GlobalSetup or GlobalCleanup.

[ig-sinicyn]

@AndreyAkinshin about return values
Afair both default toolchain and in-process toolchains do store return value using consumer.Consume() and BenchmarkAction<T>.result respectively. All we had to do is to pass these stored values back to the hosi via IHost.ReportResults()

[AndreyAkinshin]

1. The only goal of consumer.Consume() and BenchmarkAction<T>.result is preventing of DCE. I'm not sure that storing values is the best way to do it. It works ok for now, but probably I will find another way in the future (without storing values).
2. Right now the only purpose of returned values is DCE. I'm worried that if we provide other ways of using these values, it can provoke wrong benchmark design because people will try to add additional logic in the benchmark body (they should focus only on creating an isolating piece of code without any side-effects).
3. The last problem: a method can return a new value each time. Which one should we use? The first one? The last one?
   I'm pretty sure that the next feature request will be about tracking all returned values which should be aggregating somehow. It's not that benchmarking library should do, and it can spoil measurements.

[ig-sinicyn]

@AndreyAkinshin I completely do agree with all of those - the feature, even if implemented should not spoil benchmark results.

At the same time the current implementation does allow to cover at least one case for (almost) free, it looks like a low-hanging fruit for me. If this is just a coincidence and the Consumer implementation is going to change in near future - nevermind:)

[AndreyAkinshin]

The question is not about an easy way to implement it in BenchmarkDotNet. The question is about good API which doesn't provoke new problems. And it's always hard to change the API because it brakes existed benchmarks. I know, it's hard to come up with a perfect API in advance, but we should try. I think it should be consistent (if we use returned values for one purpose and other ways for all other tasks, it's not consistent), it should be obvious, it shouldn't provoke bad benchmark design.
What if I want to provide several values from a benchmark to the summary table? How can I do it via the returned values? If you use an object as a returned value, how can we handle additional memory traffic? If I return a double value, how it should be formatted? What if I have two benchmark methods which return doubles, but I want to have different formatting rules for them?

I think that it will be better to provide a common way for all additional columns. And we should do it in GlobalSetup.

[adamsitnik]

My initial idea was:

[Benchmark]
public int CompressA() => magic();

[ExtraResults(nameof(CompressA))]
public IDictionary<string, string> ExtraDataA()
    => new Dictionary<string, string>()
    {
        { "benchAcompressSize", CompressA().ToString() },
        { "anotherInterestingThing", "itsValue" }
    };

this extra method would be executed once, after stoping the watch, before last iteration/global cleanup. Then serialized and available in the results, so people could use sth like this in the columns:

public class MyColumn : IColumn
{
   public string GetValue(Summary summary, Benchmark benchmark) 
    => summary.ExtraResults[benchmark].GetValue("theKey");
}

[AndreyAkinshin]

@adamsitnik, it looks ok. Anyway, I don't have better ideas. Let's try to implement it.

[aalmada]

The solution proposed by @adamsitnik would also be a solution to #784 (?)

[lipchev]

As a workaround- I've resorted to using a Source Generator that creates a class named BenchmarkConstants where I have all of the dynamically calculated stuff (like the size of my Enums) placed as constants.