Update perf proj from arcade#38127

DrewScoggins · 2020-06-18T23:33:57Z

No description provided.

ghost · 2020-06-18T23:34:02Z

Tagging subscribers to this area: @safern, @ViktorHofer
Notify danmosemsft if you want to be subscribed.

safern · 2020-06-19T00:23:25Z

eng/common/performance/perfhelixpublish.proj

+    <HelixWorkItem Include="Crossgen2 Composite Framework R2R">
+      <PayloadDirectory>$(WorkItemDirectory)\ScenarioCorrelation</PayloadDirectory>	
+      <Command>$(Python) %HELIX_CORRELATION_PAYLOAD%\performance\src\scenarios\crossgen2\test.py crossgen2 --composite %HELIX_CORRELATION_PAYLOAD%\performance\src\scenarios\crossgen2\framework-r2r.dll.rsp --core-root %HELIX_CORRELATION_PAYLOAD%\Core_Root</Command>	
+      <Timeout>1:00</Timeout>  


Why 1 hour timeout? This leads to very long hangs blocking a machine for long periods of time.

I also see the other workitems we send have 4 hours timeouts. Can we modify this for the purpose of PR runs?

For crossgen2 composite workitem, 1 hour timeout also takes care of CI runs where we run 5 iterations, while we only have 1 iteration for PR runs and each iteration takes ~8min. For the rest of the workitems we will be able to shorten the timeout once the partition is enabled. @DrewScoggins Do you have an estimate of the timeout value after partition?

I opened: #38136 to follow up.

The reason why this concerns me is that if we enter a deadlock like: #38138 the machine is blocked for that period of time.

For the micro benchmarks we should be able to set the timeout to about 10 minutes once we get this partition change in. A run take about 3-5 minutes, but I want to leave some wiggle room. These are of course Helix timeouts.

Yeah, Helix timeouts are the ones we care about reducing on PRs for these micro benchmarks. 10-15 mins sounds good to me.

DrewScoggins · 2020-06-19T18:57:08Z

Added the timeout, but just need to make sure that it works. If it does I will check this in here and also check-in this change on the arcade side.

safern · 2020-06-19T19:06:27Z

eng/common/performance/perfhelixpublish.proj

    Partition the Microbenchmarks project, but nothing else
  -->
-  <ItemGroup Condition="$(TargetCsproj.Contains('MicroBenchmarks.csproj'))">
+  <ItemGroup Condition="$(TargetCsproj.Contains('MicroBenchmarks.csproj')) and $($HelixSourcePrefix) == 'pr'">


Typo:

Suggested change

<ItemGroup Condition="$(TargetCsproj.Contains('MicroBenchmarks.csproj')) and $($HelixSourcePrefix) == 'pr'">

<ItemGroup Condition="$(TargetCsproj.Contains('MicroBenchmarks.csproj')) and $(HelixSourcePrefix) == 'pr'">

safern · 2020-06-19T19:07:19Z

eng/common/performance/perfhelixpublish.proj

+    </HelixWorkItem>
+  </ItemGroup>
+
+  <ItemGroup Condition="$(TargetCsproj.Contains('MicroBenchmarks.csproj')) and $($HelixSourcePrefix) == 'official'">


Typo

Suggested change

<ItemGroup Condition="$(TargetCsproj.Contains('MicroBenchmarks.csproj')) and $($HelixSourcePrefix) == 'official'">

<ItemGroup Condition="$(TargetCsproj.Contains('MicroBenchmarks.csproj')) and $(HelixSourcePrefix) == 'official'">

Yeah, just found that. Doing a run to make sure it works now then going to check-in

safern · 2020-06-19T19:10:08Z

eng/common/performance/perfhelixpublish.proj

+      <PreCommands Condition="'$(Compare)' == 'true'">$(WorkItemCommand) --bdn-artifacts $(BaselineArtifactsDirectory) --bdn-arguments="--anyCategories $(BDNCategories) $(ExtraBenchmarkDotNetArguments) $(BaselineCoreRunArgument) --partition-count $(PartitionCount) --partition-index %(HelixWorkItem.Index)"</PreCommands>
+      <Command>$(WorkItemCommand) --bdn-artifacts $(ArtifactsDirectory) --bdn-arguments="--anyCategories $(BDNCategories) $(ExtraBenchmarkDotNetArguments) $(CoreRunArgument) --partition-count $(PartitionCount) --partition-index %(HelixWorkItem.Index)"</Command>
+      <PostCommands Condition="'$(Compare)' == 'true'">$(DotnetExe) run -f $(_Framework) -p $(ResultsComparer) --base $(BaselineArtifactsDirectory) --diff $(ArtifactsDirectory) --threshold 2$(Percent) --xml $(XMLResults);$(FinalCommand)</PostCommands>
+      <Timeout>45</Timeout>


It would be simpler if you introduce an intermediate variable then you wouldn't need to duplicate these code. You could just set the timeout on that variable and use it when creating the HelixWorkItem that way you would only have one HelixWorkItem declaration for Microbenchmarks

safern · 2020-06-19T20:53:24Z

eng/common/performance/perfhelixpublish.proj

    <WorkItemCommand>$(WorkItemCommand) $(CliArguments)</WorkItemCommand>
  </PropertyGroup>

+  <PropertyGroup Condition="$(HelixSourcePrefix) == 'pr'">


I think you need to also escape $(HelixSourcePrefix) for the comparison to be correct:

Suggested change

<PropertyGroup Condition="$(HelixSourcePrefix) == 'pr'">

<PropertyGroup Condition="'$(HelixSourcePrefix)' == 'pr'">

safern · 2020-06-19T20:53:30Z

eng/common/performance/perfhelixpublish.proj

+    <WorkItemTimeout>15</WorkItemTimeout>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="$(HelixSourcePrefix) == 'official'">


safern · 2020-06-19T22:57:20Z

eng/common/performance/perfhelixpublish.proj

+    <WorkItemTimeout>0:15</WorkItemTimeout>
+  </PropertyGroup>
+
+  <PropertyGroup Condition="'$(HelixSourcePrefix)'' == 'official'">


If for some reason HelixSourcePrefix is not pr nor official it will use the default HelixSDK timeout, maybe we want to be intentional about it. Something like:

<PropertyGroup> <WorkItemTimeout>0:45</WorkItemTimeout> <WorkItemTimeout Condition="'$(HelixSourcePrefix)' != 'official'">0:15</WorkItemTimeout> </PropertyGroup>

safern · 2020-06-19T22:57:41Z

eng/common/performance/perfhelixpublish.proj

    <WorkItemCommand>$(WorkItemCommand) $(CliArguments)</WorkItemCommand>
  </PropertyGroup>

+  <PropertyGroup Condition="'$(HelixSourcePrefix)'' == 'pr'">


Nit: extra '

DrewScoggins · 2020-06-22T20:58:58Z

@safern I think this is good to go.

safern

@DrewScoggins thanks for including the timeout change here. Did we validate that Helix actually honored the new timeout?

DrewScoggins · 2020-06-23T06:11:31Z

Yes, I did. It was listed at 900 seconds.

Update perf proj from arcade

9965784

Dotnet-GitSync-Bot added the area-Infrastructure-libraries label Jun 18, 2020

DrewScoggins requested a review from ooooolivia June 18, 2020 23:34

safern reviewed Jun 19, 2020

View reviewed changes

Add timeout for pr and internal jobs

2fcb7b9

safern reviewed Jun 19, 2020

View reviewed changes

Change to a variable that is set

b2bc9a2

safern reviewed Jun 19, 2020

View reviewed changes

DrewScoggins added 2 commits June 19, 2020 14:28

Escape variable with single quotes

ec3b233

Change timeout from days to minutes

223d805

safern reviewed Jun 19, 2020

View reviewed changes

Fix typo

e5ca09d

jaredpar mentioned this pull request Jun 22, 2020

OSX machines are de-provisioned during CI / PR runs leading to failures #34472

Closed

Change to default

1257340

safern approved these changes Jun 22, 2020

View reviewed changes

DrewScoggins merged commit 7c2e2e7 into dotnet:master Jun 23, 2020

safern mentioned this pull request Jun 23, 2020

Performance runs can hang for 4 hours blocking machines in the helix queue #38136

Closed

jaredpar mentioned this pull request Jun 23, 2020

PayloadGroup0 is timing out #38284

Closed

ghost locked as resolved and limited conversation to collaborators Dec 8, 2020

	<ItemGroup Condition="$(TargetCsproj.Contains('MicroBenchmarks.csproj')) and $($HelixSourcePrefix) == 'pr'">
	<ItemGroup Condition="$(TargetCsproj.Contains('MicroBenchmarks.csproj')) and $(HelixSourcePrefix) == 'pr'">

	<PropertyGroup Condition="$(HelixSourcePrefix) == 'pr'">
	<PropertyGroup Condition="'$(HelixSourcePrefix)' == 'pr'">

Comments

Conversation

DrewScoggins commented Jun 18, 2020

Uh oh!

ghost commented Jun 18, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

safern Jun 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrewScoggins commented Jun 19, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DrewScoggins commented Jun 22, 2020

Uh oh!

safern left a comment

Choose a reason for hiding this comment

Uh oh!

DrewScoggins commented Jun 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

safern Jun 19, 2020 •

edited

Loading