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

Custom SimpleJob Id ignored when the MinIterationTime attribute is used. #2473

Closed
m-kovac opened this issue Nov 30, 2023 · 2 comments · Fixed by #2491
Closed

Custom SimpleJob Id ignored when the MinIterationTime attribute is used. #2473

m-kovac opened this issue Nov 30, 2023 · 2 comments · Fixed by #2491
Labels
Milestone

Comments

@m-kovac
Copy link

m-kovac commented Nov 30, 2023

Hello, just identified the issue that causes all Jobs declared as attributes to have their Id's replaced with random Job names in the summary table if the MinIterationTime attribute is used.

Steps to reproduce:
Create benchmark class and add two SimpleJob attributes as well as AllStatisticsColumn to show the Job Id. Here I just use the default example from README.

[AllStatisticsColumn]
[SimpleJob(RunStrategy.ColdStart, launchCount: 5, warmupCount: 1, iterationCount: 1, id: "Job A")]
[SimpleJob(RunStrategy.Throughput, launchCount:1, warmupCount:1, iterationCount: 5, id: "Job B")]
public class Md5VsSha256
{
    private SHA256 sha256 = SHA256.Create();
    private MD5 md5 = MD5.Create();
    private byte[] data;

    [Params(1000, 10000)]
    public int N;

    [GlobalSetup]
    public void Setup()
    {
        data = new byte[N];
        new Random(42).NextBytes(data);
    }

    [Benchmark]
    public byte[] Sha256() => sha256.ComputeHash(data);

    [Benchmark]
    public byte[] Md5() => md5.ComputeHash(data);
}

In this case the table shows correct Job Id in the table( included only 2 lines for brevity)

Method Job IterationCount LaunchCount RunStrategy UnrollFactor N Mean Error StdDev StdErr Min Q1 Median Q3 Max Op/s
Sha256 Job A 1 5 ColdStart 1 1000 488,140.0 ns 134,559.1 ns 34,944.57 ns 15,627.69 ns 454,800.0 ns 474,900.0 ns 475,200.0 ns 489,100.0 ns 546,700.0 ns 2,048.6
Md5 Job A 1 5 ColdStart 1 1000 512,220.0 ns 101,163.3 ns 26,271.79 ns 11,749.10 ns 483,100.0 ns 487,600.0 ns 518,800.0 ns 527,100.0 ns 544,500.0 ns 1,952.3

In case when also the MinIterationTIme attribute is added, the table shows the randomly generated Job Id

[AllStatisticsColumn]
[MinIterationTime(25)]
[SimpleJob(RunStrategy.ColdStart, launchCount: 5, warmupCount: 1, iterationCount: 1, id: "Job A")]
[SimpleJob(RunStrategy.Throughput, launchCount:1, warmupCount:1, iterationCount: 5, id: "Job B")]
public class Md5VsSha256
{
   ...
}
Method Job IterationCount LaunchCount RunStrategy UnrollFactor N Mean Error StdDev StdErr Min Q1 Median Q3 Max Op/s
Sha256 Job-UJDDOK 1 5 ColdStart 1 1000 498.600 us 93.2607 us 24.2195 us 10.8313 us 483.000 us 483.500 us 487.300 us 498.800 us 540.400 us 2,005.6
Md5 Job-UJDDOK 1 5 ColdStart 1 1000 552.900 us 314.1166 us 81.5751 us 36.4815 us 485.900 us 514.700 us 526.400 us 543.600 us 693.900 us 1,808.6
@MattFromRVA
Copy link
Contributor

Leaving some findings here to (hopefully) help the next person.

I noticed that I can recreate the issue with other attributes like [IterationTime(100)] or [MinInvokeCount(2)] as long as it has .IsMutator set to true.

The ResolvedId is set in the Job class public string ResolvedId => HasValue(IdCharacteristic) ? Id : JobIdGenerator.GenerateRandomId(this);

So I assume somewhere along the way IdCharacteristic loses it's value.

One method of interest with my comments added. Found in the ImmutableConfigBuilder class:

private static IReadOnlyList<Job> GetRunnableJobs(IEnumerable<Job> jobs)
{
    var unique = jobs.Distinct(JobComparer.Instance).ToArray();
    var result = new List<Job>();

    foreach (var standardJob in unique.Where(job => !job.Meta.IsMutator && !job.Meta.IsDefault))
        result.Add(standardJob);

    var customDefaultJob = unique.SingleOrDefault(job => job.Meta.IsDefault);
    var defaultJob = customDefaultJob ?? Job.Default;

    if (!result.Any())
        result.Add(defaultJob);

    // If no mutatorJob then the ResolvedId names are kept in place as the code will skip this foreach.

    // Here is where the ResolvedId changes to the randomly generated value
    foreach (var mutatorJob in unique.Where(job => job.Meta.IsMutator))
    {
        for (int i = 0; i < result.Count; i++)
        {
	    // As soon as UnfreezeCopy() finishes the resolvedId changes
            var copy = result[i].UnfreezeCopy();

            copy.Apply(mutatorJob);

            result[i] = copy.Freeze();
        }
    }

    return result;
}

I also kept testing old commits and the issue was there as well. I went as far as 7/26/23 d93490d and this was still an issue.

@MattFromRVA
Copy link
Contributor

@m-kovac @timcassell Ok I figured it out. PR submitted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants