Don't set affinity for multithreaded perf tests

[AndyAyersMS]

The profile for affinitized multithreaded tests may diverge from the un-affinitized profile. In particular, affinitizing can shift the focus of the perf away from jit codegen. See for instance #9838.

We should remove affinity from the following jit perf tests:

binarytrees-5
fannkuch-redux-5
fasta-1
k-nucleotide-9
mandelbrot-7
regex-redux-5
spectralnorm-3
FractalPerf
ConsoleMandel
RayTracer
cscbench

cc @jorive

category:testing
theme:benchmarks
skill-level:intermediate
cost:small

[danmoseley]

As you know the Q6600 they run the first 7 on is 4 cores. Is it intersting to limit those tests to 4 cores?

[AndyAyersMS]

Maybe? There are a couple of different angles to this.

Our perf HW is 8 core, and it looks like trying to make it simulate having fewer cores can alter the profile enough that it may not be that interesting, at least to us jit developers.

If we modified the tests that looked at the total core count to determine task distribution to instead look at the number of available cores then we might fare better with this kind of scale down.

But if we care to track actual BenchmarkGames perf we should just run on the matching HW as other important details will differ too.

[adamsitnik]

I will make sure that it's not an issue during the port to BenchmarkDotNet.

[adamsitnik]

@jorive has an interesting idea (please keep in mind that with BDN we run every benchmark in a separate process):

1. the parent benchmark runner process should be affinitized to a single core
2. all multithreaded perf tests should be affinitized to all cores except the benchmak runner one and spawn not Env.ProcessorCount tasks/threads but Env.ProcessorCount - 1

I was wondering if it would be possible to:

1. somehow tell the OS to be "quiet" by enabling Game Mode or something similar (I would need to talk to somebody from Windows team about this, maybe @brianrob or @vancem could point me to the right person)
2. affinitize the benchmark runner process to a single core, but:
3. set affinity for multithreaded perf tests to all cores
4. for the time of running multithreaded perf test put benchmark runner to sleep to not use any CPU

@AndyAyersMS @jorive I also need to check how allocation-heavy benchmarks work with affinity set to a single core.

[brianrob]

@adamsitnik, is the benchmark runner process heavy enough that it needs its own core to avoid interfering with the benchmark itself?

I'm not aware of any quiet modes that can be used in Windows. I had this conversation with someone on the Windows Perf team a few years back. Generally, we should (and do) set the machine power configuration to "High Performance" and then after that if you want to quiet things down it's a game of whack-a-mole and the work required will vary based on machine-spec/OS/BIOS/driver configuration.

One thing that we should be careful of is doing too much affinity work as we'd like to make sure that we're not missing anything (by default people don't affinitize their apps).

[adamsitnik]

is the benchmark runner process heavy enough that it needs its own core to avoid interfering with the benchmark itself?

@brianrob it's not heavy, all it does is reading the output from the child process

while ((line = process.StandardOutput.ReadLine()) != null)
{
   results.Add(Parse(line));
}

[vancem]

My motto on things like this is 'less is more'. In general the more the benchmark is like the actual scenario you care about the better, and when you deviate from it (e.g. setting thread affinity), you have to have a very clear idea why you are doing it (because you may end up doing more harm then good.

The title of this is a recommendation to remove thread affinity which is a step toward making the benchmark more like the real scneario (which presumably does not set it), so that is good.

But I don't properly understnad the motivation for setting the thread affinity originally. Presumably it was to measure at different CPU counts. What we have discovered is that that is not without its own perturbation. Thus we get into the question of what our goals are (something @AndyAyersMS was mentioning above).

Presumably the biggests reason for these tests is to track the external bechmarking that is done (to spot regressions faster). This could be done without any affinity, and thus the request to remove it is reasonble. But before we do that we should confirm if we have other goals as well, the conditions the external benchmark runs under, and detemrine if we run any other variations of these benchmarks.

But on the whole I like the removal (because it SEEMS that it is miniumum perturbation).

[BruceForstall]

@adamsitnik Is this issue still valid? If so, should it be moved to the performance repo and closed in coreclr?

[adamsitnik]

@BruceForstall after moving to the performance repo and BenchmarkDotNet we don't set the affinity anymore (for all benchmarks).