Revise how we set tiering and pgo options for spmi benchmark collections #87292
Conversation
Don't set these in the BDN environment; set them via the BDN command line so they only impact the process being benchmarked. Fixes dotnet#86410
dotnet-issue-labeler
bot
added
the
area-CodeGen-coreclr
|
Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch Issue DetailsDon't set these in the BDN environment; set them via the BDN command line so they only impact the process being benchmarked. Fixes #86410
|
|
@kunalspathak PTAL Internal pipeline run with this new script: green save unrelated OSX failure. https://dev.azure.com/dnceng/internal/_build/results?buildId=2196191&view=results |
| "--iterationCount 1 --warmupCount 0 --invocationCount 1 --unrollFactor 1 --strategy ColdStart --logBuildOutput " \ | ||
| f"--envVars DOTNET_JitName:{shim_name} DOTNET_ZapDisable:1 DOTNET_ReadyToRun:0" | ||
|
|
||
| if coreclr_args.tiered_pgo: |
There was a problem hiding this comment.
do you mind also pulling the --iterationCount 1 --warmupCount 0 --invocationCount 1 --unrollFactor 1 --strategy ColdStart in common case?
Also, shouldn't you be passing iterationCount, etc. if for tiered/pgo runs? And a note about DOTNET_ZapDisable:1 DOTNET_ReadyToRun:0 too. Do we still want them for tiered/pgo runs?
There was a problem hiding this comment.
There is no common case right now, are you saying I should factor the common aspects out (or at least the common trailing elements)?
I wasn't planning on changing how we collect at this point. I would have to study the impact of altering BDN strategy to see if it adds value.
There was a problem hiding this comment.
There is no common case right now, are you saying I should factor the common aspects out (or at least the common trailing elements)?
Yes, that's what I meant.
I wasn't planning on changing how we collect at this point. I would have to study the impact of altering BDN strategy to see if it adds value.
Sure, we can limit this PR for just the changes you have currently and rethink about longer term strategy of how we should collect them in a later PR.
ghost
added
needs-author-action
Typically to test out things, I comment out all the unrelated collections in yml file and that way we just do the collection for type we are testing for. E.g. https://dev.azure.com/dnceng/internal/_build/results?buildId=2193577&view=results that just ran real-world collection. |
|
https://dev.azure.com/dnceng/internal/_build/results?buildId=2196878&view=results is the revised run -- benchmark runs were all happy, though a test build failed (will restart it once I am able). |
Don't set these in the BDN environment; set them via the BDN command line so they only impact the process being benchmarked.
Fixes #86410