Change how comparisons are made

We now have two options:

- Compare to the same environment/type for a different commit
- Compare with the same commit/type for all matching environments

src/NodaTime.Web/Controllers/BenchmarksController.cs

@@ -32,24 +32,41 @@ public BenchmarksController(IBenchmarkRepository repository)
         public IActionResult ViewType(string typeId)
         {
             var type = repository.GetType(typeId);
-            var previousRun = GetPreviousRun(type.Run);
-            var previousRunType = previousRun?.Types_.FirstOrDefault(t => t.FullTypeName == type.FullTypeName);
-            IEnumerable<BenchmarkType> comparisonTypes = repository
-                .GetTypesByCommitAndType(type.Run.Commit, type.FullTypeName)
-                .Where(t => t != type)
-                .OrderBy(t => t.Environment.Machine)
-                .ThenBy(t => t.Environment.TargetFramework)
-                .ThenBy(t => t.Environment.RuntimeVersion)
+            var previousCommit = GetPreviousRun(type.Run)?.Commit;
+            return View((type, previousCommit));
+        }
+
+        [Route("/benchmarks/types/{typeId}:compareEnvironments")]
+        public IActionResult CompareTypesByEnvironment(string typeId)
+        {
+            var left = repository.GetType(typeId);
+            var runs = repository.ListEnvironments()
+                .Select(e => e.Runs.FirstOrDefault(r => r.Commit == left.Run.Commit))
+                .Where(r => r != null && r != left.Run)
+                .Select(r => r.Types_.FirstOrDefault(t => t.FullTypeName == left.FullTypeName))
                 .ToList();
-            return View((type, previousRunType, comparisonTypes));
+            // Always make the selected run the first one.
+            runs.Insert(0, left);
+            return View(new CompareTypesByEnvironmentViewModel(runs));
         }
 
-        [Route("/benchmarks/types/{leftTypeId}/compare/{rightTypeId}")]
-        public IActionResult CompareTypes(string leftTypeId, string rightTypeId)
+
+        [Route("/benchmarks/types/{leftTypeId}:compareWithCommit")]
+        public IActionResult CompareTypesByCommit(string leftTypeId, [FromQuery] string commit)
         {
-            var left = repository.GetType(leftTypeId);
-            var right = repository.GetType(rightTypeId);
-            return View(new CompareTypesViewModel(left, right));
+            var leftType = repository.GetType(leftTypeId);
+            var environment = leftType.Environment;
+            var run = environment.Runs.FirstOrDefault(r => r.Commit == commit);
+            if (run == null)
+            {
+                return NotFound();
+            }
+            var rightType = run.Types_.FirstOrDefault(t => t.FullTypeName == leftType.FullTypeName);
+            if (rightType == null)
+            {
+                return NotFound();
+            }
+            return View(new CompareTypesByCommitViewModel(leftType, rightType));
         }
 
         [Route("/benchmarks/benchmarks/{benchmarkId}")]

src/NodaTime.Web/Models/BenchmarkProtoPartials.cs

@@ -38,4 +38,12 @@ internal void PopulateLinks()
             }
         }
     }
+
+    public partial class BenchmarkEnvironment
+    {
+        public string BriefOperatingSystem =>
+            OperatingSystem.StartsWith("Ubuntu") || OperatingSystem.Contains("Linux") ? "Linux"
+            : OperatingSystem.Contains("Microsoft Windows") ? "Windows"
+            : OperatingSystem;
+    }
 }

src/NodaTime.Web/Models/GoogleStorageBenchmarkRepository.cs

@@ -62,8 +62,7 @@ private CacheValue(
                 string environmentCrc32c,
                 Dictionary<string, BenchmarkRun> runsByStorageName)
             {
-                // Clone the environments to avoid mutating one that might still be being used elsewhere.
-                Environments = environments.Select(e => e.Clone()).ToList();
+                Environments = environments;
 
                 this.environmentCrc32c = environmentCrc32c;
                 this.runsByStorageName = runsByStorageName;
@@ -95,7 +94,7 @@ public static CacheValue Refresh(CacheValue previous, StorageClient client)
                 var environmentObject = client.GetObject(BucketName, EnvironmentObjectName);
                 var environments = environmentObject.Crc32c == previous.environmentCrc32c
                     ? previous.Environments.Select(env => env.Clone()).ToList()
-                    : LoadEnvironments(client);
+                    : LoadEnvironments(client).OrderBy(e => e.Machine).ThenBy(e => e.TargetFramework).ThenBy(e => e.OperatingSystem).ToList();
 
                 // Don't just use previous.runsByStorageName blindly - some may have been removed.
                 var runsByStorageName = new Dictionary<string, BenchmarkRun>();

src/NodaTime.Web/ViewModels/CompareTypesViewModel.cs → src/NodaTime.Web/ViewModels/CompareTypesByCommitViewModel.cs

@@ -11,17 +11,15 @@
 namespace NodaTime.Web.ViewModels
 {
     /// <summary>
-    /// View-model for comparing two runs of the same type (by fully-qualified name).
+    /// View-model for comparing two runs of the same type, in the same environment, at different commits.
     /// </summary>
-    public class CompareTypesViewModel
+    public class CompareTypesByCommitViewModel
     {
-
-        public bool SingleEnvironment => Left.Environment.BenchmarkEnvironmentId == Right.Environment.BenchmarkEnvironmentId;
-
+        public BenchmarkEnvironment Environment => Left.Environment;
         public BenchmarkType Left { get; }
         public BenchmarkType Right { get; }
 
-        public CompareTypesViewModel(BenchmarkType left, BenchmarkType right)
+        public CompareTypesByCommitViewModel(BenchmarkType left, BenchmarkType right)
         {
             Left = left;
             Right = right;

src/NodaTime.Web/ViewModels/CompareTypesByEnvironmentViewModel.cs

@@ -0,0 +1,56 @@
+// Copyright 2017 The Noda Time Authors. All rights reserved.
+// Use of this source code is governed by the Apache License 2.0,
+// as found in the LICENSE.txt file.
+using MoreLinq;
+using NodaTime.Benchmarks;
+using NodaTime.Web.Helpers;
+using System;
+using System.Collections.Generic;
+using System.Linq;
+
+namespace NodaTime.Web.ViewModels
+{
+    /// <summary>
+    /// View-model for comparing multiple runs of the same type at the same commit.
+    /// </summary>
+    public class CompareTypesByEnvironmentViewModel
+    {
+        public IList<BenchmarkType> Types { get; }
+        public BenchmarkType TargetType { get; }
+
+        public CompareTypesByEnvironmentViewModel(IList<BenchmarkType> types)
+        {
+            Types = types;
+            TargetType = Types.First();
+        }
+
+        // TODO: Handle missing benchmarks
+        public IEnumerable<(Benchmark description, IEnumerable<Statistics> statistics)> GetBenchmarks()
+        {
+            List<Dictionary<string, Statistics>> statisticsByNameInTypeOrder = Types
+                .Select(t => t.Benchmarks.ToDictionary(tb => tb.Method, tb => tb.Statistics))
+                .ToList();
+            return TargetType.Benchmarks
+                .OrderBy(b => b.Method)
+                .Select(b => (b, statisticsByNameInTypeOrder.Select(d => d.GetValueOrDefault(b.Method))));
+        }
+
+        /* TODO: Reinstate this, on a per cell basis 
+        private static bool IsImportant(Statistics left, Statistics right)
+        {
+            if (left == null || right == null)
+            {
+                return false;
+            }
+            // Just in case we have strange results that might cause problems on division. (If something takes
+            // less than a picosecond, I'm suspicious...)
+            if (left.Mean < 0.001 || right.Mean < 0.001)
+            {
+                return true;
+            }
+            double min = Math.Min(left.Mean, right.Mean);
+            double max = Math.Max(left.Mean, right.Mean);
+            return min / max < 0.8; // A 20% change either way is important. (Arbitrary starting point...)
+        } */
+    }
+}

src/NodaTime.Web/Views/Benchmarks/CompareTypesByCommit.cshtml

@@ -0,0 +1,44 @@
+@using NodaTime.Web.ViewModels;
+@model CompareTypesByCommitViewModel
+
+@{
+    ViewBag.Title = "Benchmark type comparison";
+}
+
+<div class="row">
+    <div>
+        <h1>Benchmark type comparison for @Model.Left.FullTypeName</h1>
+    </div>
+    <div>
+        Environment: <a href="/benchmarks/environments/@Model.Environment.BenchmarkEnvironmentId">@Model.Environment.Machine, @Model.Environment.TargetFramework</a><br />
+        Left run: <a href="/benchmarks/runs/@Model.Left.Run.BenchmarkRunId">@Model.Left.Run.BenchmarkRunId.TruncateGuid()</a>
+            (@Html.RenderTimestamp(@Model.Left.Run.Start));
+            commit <a href="https://github.com/nodatime/nodatime/commit/@Model.Left.Run.Commit">@Model.Left.Run.Commit.TruncateCommit()</a><br />
+        Right run: <a href="/benchmarks/runs/@Model.Right.Run.BenchmarkRunId">@Model.Right.Run.BenchmarkRunId.TruncateGuid()</a>
+            (@Html.RenderTimestamp(@Model.Right.Run.Start));
+            commit <a href="https://github.com/nodatime/nodatime/commit/@Model.Right.Run.Commit">@Model.Right.Run.Commit.TruncateCommit()</a><br />
+    </div>
+    <div>
+        <h2>Benchmarks</h2>
+    </div>
+    <div>
+        <table>
+            @* TODO: Parameters, if we ever use them. *@
+            <tr>
+                <th>Method</th>
+                <th>Mean time (left)</th>
+                <th>Mean time (right)</th>
+            </tr>
+            @foreach (var item in Model.GetBenchmarks())
+            {
+                <tr class="@(item.important ? "benchmark-important": "")">
+                    <td>@item.description.Method</td>
+                    <td>@Html.RenderTime(item.left?.Mean)</td>
+                    <td>@Html.RenderTime(item.right?.Mean)</td>
+                </tr>
+            }
+        </table>
+    </div>
+    @* Work out how to style the above to leave some padding instead... *@
+    <p />
+</div>

src/NodaTime.Web/Views/Benchmarks/CompareTypesByEnvironment.cshtml

@@ -0,0 +1,44 @@
+@using NodaTime.Web.ViewModels;
+@model CompareTypesByEnvironmentViewModel
+
+@{
+    ViewBag.Title = "Benchmark type comparison";
+}
+
+<div class="row">
+    <div>
+        <h1>Benchmark environment comparison for @Model.TargetType.FullTypeName</h1>
+    </div>
+    <div>
+        <p>
+            Type @Model.TargetType.FullTypeName at commit <a href="https://github.com/nodatime/nodatime/commit/@Model.TargetType.Run.Commit">@Model.TargetType.Run.Commit.TruncateCommit()</a>
+        </p>
+    </div>
+    <div>
+        <h2>Benchmarks</h2>
+    </div>
+    <div>
+        <table>
+            @* TODO: Parameters, if we ever use them. *@
+            <tr>
+                <th>Method</th>
+                @foreach (var type in Model.Types)
+                {
+                    <th><a href="/benchmarks/types/@type.BenchmarkTypeId">@type.Environment.Machine<br />@type.Environment.TargetFramework<br />@type.Environment.BriefOperatingSystem</a></th>
+                }
+            </tr>
+            @foreach (var item in Model.GetBenchmarks())
+            {
+                <tr>
+                    <td>@item.description.Method</td>
+                    @foreach (var stat in item.statistics)
+                    {
+                      <td>@Html.RenderTime(stat?.Mean)</td>
+                    }
+                </tr>
+            }
+        </table>
+    </div>
+    @* Work out how to style the above to leave some padding instead... *@
+    <p />
+</div>

src/NodaTime.Web/Views/Benchmarks/ViewType.cshtml

@@ -1,14 +1,13 @@
 @using NodaTime.Benchmarks;
-@model (BenchmarkType type, BenchmarkType previousRunType, IEnumerable<BenchmarkType> comparisonEnvironments)
+@model (BenchmarkType type, string previousCommit)
 
 @{
     ViewBag.Title = "Benchmark type";
     var type = Model.type;
     var run = type.Run;
     var env = type.Environment;
     var benchmarks = type.Benchmarks;
-    var previousRunType = Model.previousRunType;
-    var comparisonEnvironments = Model.comparisonEnvironments;
+    var previousCommit = Model.previousCommit;
 }
 
 <div class="row">
@@ -20,28 +19,15 @@
             Type: @type.FullTypeName<br />
             Environment: <a href="/benchmarks/environments/@env.BenchmarkEnvironmentId">@env.Machine, @env.TargetFramework</a><br />
             Run: <a href="/benchmarks/runs/@run.BenchmarkRunId">@run.BenchmarkRunId.TruncateGuid()</a><br />
-            Compare with:
-            <ul>
-                @if (previousRunType != null)
-                {
-                    <li><a href="/benchmarks/types/@type.BenchmarkTypeId/compare/@previousRunType.BenchmarkTypeId">Compare with previous run</a></li>
-                }
-                else
-                {
-                    <li>(No previous run to compare this with.)</li>
-                }
-                @if (comparisonEnvironments.Any())
-                {
-                    @foreach (var comparison in comparisonEnvironments)
-                    {
-                        <li><a href="/benchmarks/types/@type.BenchmarkTypeId/compare/@comparison.BenchmarkTypeId">Same commit on @comparison.Environment.Machine (@comparison.Environment.TargetFramework)</a></li>
-                    }
-                }
-                else
-                {
-                    <li>(No other environments to compare this with.)</li>
-                }
-            </ul>
+            @if (previousCommit != null)
+            {
+                <a href="/benchmarks/types/@type.BenchmarkTypeId:compareWithCommit?commit=@previousCommit">Compare with previous commit in this environment</a><br />
+            }
+            else
+            {
+                @:(No previous commit to compare this with.)<br />
+            }
+            <a href="/benchmarks/types/@type.BenchmarkTypeId:compareEnvironments">Compare with same type and commit in other environments</a>
         </p>
     </div>
     <div>

src/NodaTime.Web/wwwroot/css/site.css

@@ -406,7 +406,8 @@ td.pattern-example {
   width: 30%
 }
 
-.benchmark-important > td {
+/* TODO: Can we do better than this? */
+.benchmark-important > td, .benchmark-important td {
   color: red;
   font-weight: bold
-}
+}