Updating version of xUnit-Performance-Api #11987

jorive · 2017-05-30T20:03:08Z

This update contains a fix for this issue: Virtual getter for Benchmark.InnerIterationCount adds overhead microsoft/xunit-performance#230
Consolidate the Api version in two places: dependencies.prop and PerfHarness.csproj
Move performance log files to root directory where Jenkins can archive them

NOTE: This change will reflect on some benchmarks improving their timings

- This update contains a fix for this issue: microsoft/xunit-performance#230 - Consolidate the Api version in two places: `dependencies.prop` and `PerfHarness.csproj` - Move performance log files to root directory where Jenkins can archive them

wtgodbe · 2017-05-30T20:07:00Z

tests/src/Common/PerfHarness/PerfHarness.csproj

-    <PackageReference Include="xunit.performance.api">
-      <Version>1.0.0-beta-build0006</Version>
-    </PackageReference>
+    <PackageReference Include="xunit.performance.api" Version="1.0.0-beta-build0007" />


I don't think this works with all versions of msbuild - @weshaggard can you comment?

Correct this will only work if you load this project with the new msbuild 15 or msbuild core that comes with the SDK. If someone tries to load this project in VS2015 or msbuild <15 then it will blow-up. It might not be an issue for this particular project but something that needs to be kept in mind.

@weshaggard Is there a way to import dir.props and dir.targets in the new csproj file? I would like to have the version of the package in a central place. Is that a supported scenario?

Yes you can still do an import the same way as you used to be able to <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />

If I add Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" /> to the PerfHarness.csproj, then I can restore, but I cannot publish. What am I missing?

$ "%CORECLR_ROOT%Tools\dotnetcli\dotnet.exe" --info .NET Command Line Tools (2.0.0-preview1-005724) Product Information: Version: 2.0.0-preview1-005724 Commit SHA-1 hash: e391b5d5f3 Runtime Environment: OS Name: Windows OS Version: 10.0.15063 OS Platform: Windows RID: win10-x64 Base Path: %CORECLR_ROOT%\Tools\dotnetcli\sdk\2.0.0-preview1-005724\ $ "%CORECLR_ROOT%\Tools\dotnetcli\dotnet.exe" restore "%CORECLR_ROOT%\tests\src\Common\PerfHarness\PerfHarness.csproj" %CORECLR_ROOT%\tests\src\Common\PerfHarness\PerfHarness.csproj Restoring packages for %CORECLR_ROOT%\tests\src\Common\PerfHarness\PerfHarness.csproj... Lock file has not changed. Skipping lock file write. Path: %CORECLR_ROOT%\tests\src\Common\PerfHarness\obj\project.assets.json Restore completed in 280.64 ms for %CORECLR_ROOT%\tests\src\Common\PerfHarness\PerfHarness.csproj. NuGet Config files used: %CORECLR_ROOT%\tests\src\NuGet.Config %APPDATA%\NuGet\NuGet.Config C:\Program Files (x86)\NuGet\Config\Microsoft.VisualStudio.Offline.config Feeds used: https://dotnet.myget.org/F/dotnet-core/api/v3/index.json https://www.myget.org/F/nugetbuild/api/v3/index.json https://api.nuget.org/v3/index.json $ "%CORECLR_ROOT%\Tools\dotnetcli\dotnet.exe" publish "%CORECLR_ROOT%\tests\src\Common\PerfHarness\PerfHarness.csproj" -c Release -o "%CORECLR_ROOT%\sandbox" Microsoft (R) Build Engine version 15.2.93.5465 Copyright (C) Microsoft Corporation. All rights reserved. %CORECLR_ROOT%\Tools\Microsoft.CSharp.Core.targets(106,11): error MSB4064: The "OverrideToolHost" parameter is not supported by the "Csc" task. Verify the parameter exists on the task, and it is a settable public instance property. [%CORECLR_ROOT%\tests\src\Common\PerfHarness\PerfHarness.csproj] %CORECLR_ROOT%\Tools\Microsoft.CSharp.Core.targets(67,5): error MSB4063: The "Csc" task could not be initialized with its input parameters. [%CORECLR_ROOT%\tests\src\Common\PerfHarness\PerfHarness.csproj]

AndyAyersMS

Other than the question Will raised about the one project file edit, LGTM.

- Removed unused variable - Updated the RunId to contain the "Profile" flag because we are not copying/renaming the output files anymore - Fixed the output to console, so it is easier to see on the Jenkins job the sccript steps live and get an exact repro step if needed

dpodder

Correctness-wise LGTM, just a few stylistic questions.

dpodder · 2017-05-31T18:55:16Z

tests/scripts/run-xunit-perf.cmd

 rem   output is redirected). This can be useful to provide information on where
 rem   the script is executing.
 rem ****************************************************************************
-  if defined _debug (


Could we delete the entire :print_to_console function? I don't think anybody uses it anymore, right?

I should be reusing that code. I'll fix it.

dpodder · 2017-05-31T18:56:35Z

tests/scripts/run-xunit-perf.cmd


+  echo/
+  echo/%USERNAME%@%COMPUTERNAME% "%CD%"
+  echo/[%DATE%][%TIME:~0,-3%] $ !LV_CMD!


It looks like :run_cmd already prints this out. Doesn't this cause us to just print everything twice?

No in this case, because we are redirecting to a log file.

Ah... d'oh... I missed the obvious. Looks good.

- Mistakenly removed the CORE_ROOT environment variable because it was not used anywhere on the script, but it turns out that it is needed by some benchmarks.

jorive · 2017-05-31T21:08:13Z

test ci please
@dotnet-bot test Windows_NT x64 perf
@dotnet-bot test Windows_NT x86 perf
@dotnet-bot test Ubuntu14.04 perf

jorive · 2017-05-31T22:42:19Z

@dotnet-bot test Windows_NT x64 perf

AndyAyersMS · 2017-06-01T16:14:20Z

@jorive did you see performance shifts in some tests? Can you summarize?

jorive · 2017-06-01T16:21:31Z

Yes, I did see some performance shifts. Some benchmarks improved timings and some got slower. I'll send you links to the results...
Looking some more, it seems that those changes were within the normal noise.
Please also keep in mind that only the tests\src\performance\perflab\* and JIT\Performance\CodeQuality\Span\* are the only ones using Benchmark.InnerIterationCount

- This update contains a fix for this issue: microsoft/xunit-performance#230 - Consolidate the Api version in two places: `dependencies.prop` and `PerfHarness.csproj` - Move performance log files to root directory where Jenkins can archive them - Fixed the output to console, so it is easier to see on the Jenkins job the sccript steps live and get an exact repro step if needed

* Enable linux perf runs (#11905) - Removing `[assembly: MeasureInstructionsRetired]` from performance tests. - Due to this issue: microsoft/xunit-performance#231, we are blocked from running on performance tests on Linux. The tag is not used by infrastructure and it is passed during runtime when profiling on Windows. - Adding the option to execute CoreRun with the stability prefix. - Moving file to be archived by Jenkins and updating groovy file. - Setting the maximum number of iterations. * Updating version of xUnit-Performance-Api (#11987) - This update contains a fix for this issue: microsoft/xunit-performance#230 - Consolidate the Api version in two places: `dependencies.prop` and `PerfHarness.csproj` - Move performance log files to root directory where Jenkins can archive them - Fixed the output to console, so it is easier to see on the Jenkins job the sccript steps live and get an exact repro step if needed

Updating version of xUnit-Performance-Api

a8f5794

- This update contains a fix for this issue: microsoft/xunit-performance#230 - Consolidate the Api version in two places: `dependencies.prop` and `PerfHarness.csproj` - Move performance log files to root directory where Jenkins can archive them

jorive requested review from dpodder, brianrob, DrewScoggins, russellhadley, AndyAyersMS, wtgodbe and valenis May 30, 2017 20:03

dnfclas added the cla-already-signed label May 30, 2017

wtgodbe reviewed May 30, 2017

View reviewed changes

AndyAyersMS approved these changes May 30, 2017

View reviewed changes

José Rivero added 3 commits May 30, 2017 16:40

Removing the copy step from script and archive directly from sandbox.

1f5add7

Changes for easy CI debugging.

9bba3d5

dpodder reviewed May 31, 2017

View reviewed changes

José Rivero added 3 commits May 31, 2017 12:35

Consolidating duplicated lines of code.

27eba76

Updating function header to reflect current behavior.

c1f5568

Bug fix

ffd8358

- Mistakenly removed the CORE_ROOT environment variable because it was not used anywhere on the script, but it turns out that it is needed by some benchmarks.

jorive merged commit 1d2e6f8 into dotnet:master Jun 1, 2017

jorive deleted the dev/slimmer-xunit branch June 1, 2017 15:56

karelz modified the milestone: 2.1.0 Aug 28, 2017

Updating version of xUnit-Performance-Api #11987

Updating version of xUnit-Performance-Api #11987

Uh oh!

Conversation

jorive commented May 30, 2017

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!

AndyAyersMS left a comment

Choose a reason for hiding this comment

Uh oh!

dpodder left a comment

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!

jorive commented May 31, 2017

Uh oh!

jorive commented May 31, 2017

Uh oh!

AndyAyersMS commented Jun 1, 2017

Uh oh!

jorive commented Jun 1, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

jorive commented Jun 1, 2017 •

edited

Loading