JetStream should have a more rational story for jitter-oriented latency tests
authorfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Jun 2015 18:44:50 +0000 (18:44 +0000)
committerfpizlo@apple.com <fpizlo@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Jun 2015 18:44:50 +0000 (18:44 +0000)
https://bugs.webkit.org/show_bug.cgi?id=145762

Reviewed by Geoffrey Garen.

JetStream has some latency tests that are meant to measure jitter.  Prior to this change, they
did this by computing the RMS.  But the RMS is a pretty bad metric.  The thing that it rewards
isn't really the thing that you'd want your browser to do.  These RMS-based tests involve taking
the geomean of the RMS of some samples and the sample average.  The lower the geomean, the better
(in the JetStream harness we then invert the scores so that higher is better, but let's ignore
that for this discussion and assume that lower is better).  Here's an example of how this can go
bad.  A browser that always computes a task in some horribly long time (say, 1000ms) but never
varies that time will perform better than a browser that usually computes the task super quickly
(say, 10ms) and sometimes just a little bit less quickly (say, 15ms).  The former browser will
have an RMS of 0 and an average of 1000.  The latter will have a RMS somewhere around 3.5 and an
average of 12.5 (assuming equal probability of 10ms and 15ms).  The geomean of (0, 1000) is 0.
The geomean of (3.5, 12.5) is 6.6.  Lower is better, so the former browser scores higher - even
though it's obviously never better to have a browser always complete a task in 1000ms when a
different browser can do it in 15ms in the worst case.

JetStream should not have this pathology.  The right way of avoiding it is to replace RMS with
some other metric of how bad things get.  A good metric is the average of the worst percentile.
The worst 1% or the worst 5% would be good things to average.  This will catch cases where the VM
jittered due to JIT or GC, but it never have the pathology that we end up giving the better score
to a VM whose best case is worst than another VM's worst case.

For now, this change uses the highest samples above the 95% percentile. I'm not yet sure if that
is the best thing - it might include too many scores that are around the best-case performance -
but it's certainly better than RMS and it might be good enough to keep. But because of that
uncertainty, I'm setting the version to be "1.1-alpha1" to indicate that we aren't ready to
release this yet.

* JetStream/Octane2/base.js:
(.this.Setup.setup.setup):
(.this.TearDown.tearDown.tearDown):
(BenchmarkSuite.GeometricMeanTime):
(BenchmarkSuite.AverageAbovePercentile):
(BenchmarkSuite.GeometricMeanLatency):
(BenchmarkSuite.prototype.NotifyStep):
(BenchmarkSuite.prototype.RunSingleBenchmark):
* JetStream/Octane2/mandreel.js:
(setupMandreel):
(updateMandreelStats):
(startMandreelTimer):
(latencyMandreel):
(tearDownMandreel):
(RMSMandreel): Deleted.
* JetStream/Octane2/splay.js:
(GenerateKey):
(SplayUpdateStats):
(InsertNewNode):
(SplayTearDown):
(SplayRMS): Deleted.
* JetStream/create.rb:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@185425 268f45cc-cd09-0410-ab3c-d52691b4dbfc

12 files changed:
PerformanceTests/ChangeLog
PerformanceTests/JetStream/Octane2/base.js
PerformanceTests/JetStream/Octane2/mandreel.js
PerformanceTests/JetStream/Octane2/splay.js
PerformanceTests/JetStream/Reference.js
PerformanceTests/JetStream/create.rb
Source/JavaScriptCore/bytecode/CodeBlock.h
Source/JavaScriptCore/bytecode/DFGExitProfile.cpp
Source/JavaScriptCore/bytecode/DFGExitProfile.h
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/runtime/Options.h
Tools/Scripts/run-jsc-benchmarks

index 73af809..2794362 100644 (file)
@@ -1,3 +1,60 @@
+2015-06-08  Filip Pizlo  <fpizlo@apple.com>
+
+        JetStream should have a more rational story for jitter-oriented latency tests
+        https://bugs.webkit.org/show_bug.cgi?id=145762
+
+        Reviewed by Geoffrey Garen.
+        
+        JetStream has some latency tests that are meant to measure jitter.  Prior to this change, they
+        did this by computing the RMS.  But the RMS is a pretty bad metric.  The thing that it rewards
+        isn't really the thing that you'd want your browser to do.  These RMS-based tests involve taking
+        the geomean of the RMS of some samples and the sample average.  The lower the geomean, the better
+        (in the JetStream harness we then invert the scores so that higher is better, but let's ignore
+        that for this discussion and assume that lower is better).  Here's an example of how this can go
+        bad.  A browser that always computes a task in some horribly long time (say, 1000ms) but never
+        varies that time will perform better than a browser that usually computes the task super quickly
+        (say, 10ms) and sometimes just a little bit less quickly (say, 15ms).  The former browser will
+        have an RMS of 0 and an average of 1000.  The latter will have a RMS somewhere around 3.5 and an
+        average of 12.5 (assuming equal probability of 10ms and 15ms).  The geomean of (0, 1000) is 0.
+        The geomean of (3.5, 12.5) is 6.6.  Lower is better, so the former browser scores higher - even
+        though it's obviously never better to have a browser always complete a task in 1000ms when a
+        different browser can do it in 15ms in the worst case.
+
+        JetStream should not have this pathology.  The right way of avoiding it is to replace RMS with
+        some other metric of how bad things get.  A good metric is the average of the worst percentile.
+        The worst 1% or the worst 5% would be good things to average.  This will catch cases where the VM
+        jittered due to JIT or GC, but it never have the pathology that we end up giving the better score
+        to a VM whose best case is worst than another VM's worst case.
+        
+        For now, this change uses the highest samples above the 95% percentile. I'm not yet sure if that
+        is the best thing - it might include too many scores that are around the best-case performance -
+        but it's certainly better than RMS and it might be good enough to keep. But because of that
+        uncertainty, I'm setting the version to be "1.1-alpha1" to indicate that we aren't ready to
+        release this yet.
+
+        * JetStream/Octane2/base.js:
+        (.this.Setup.setup.setup):
+        (.this.TearDown.tearDown.tearDown):
+        (BenchmarkSuite.GeometricMeanTime):
+        (BenchmarkSuite.AverageAbovePercentile):
+        (BenchmarkSuite.GeometricMeanLatency):
+        (BenchmarkSuite.prototype.NotifyStep):
+        (BenchmarkSuite.prototype.RunSingleBenchmark):
+        * JetStream/Octane2/mandreel.js:
+        (setupMandreel):
+        (updateMandreelStats):
+        (startMandreelTimer):
+        (latencyMandreel):
+        (tearDownMandreel):
+        (RMSMandreel): Deleted.
+        * JetStream/Octane2/splay.js:
+        (GenerateKey):
+        (SplayUpdateStats):
+        (InsertNewNode):
+        (SplayTearDown):
+        (SplayRMS): Deleted.
+        * JetStream/create.rb:
+
 2015-06-03  Zalan Bujtas  <zalan@apple.com>
 
         Skip Dromaeo/jslib-modify-prototype.html for now.
index 47733f7..e5119c4 100644 (file)
@@ -1,4 +1,5 @@
 // Copyright 2013 the V8 project authors. All rights reserved.
+// Copyright (C) 2015 Apple Inc. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
@@ -46,14 +47,14 @@ performance.now = (function() {
 // arguments are functions that will be invoked before and after
 // running the benchmark, but the running time of these functions will
 // not be accounted for in the benchmark score.
-function Benchmark(name, doWarmup, doDeterministic, run, setup, tearDown, rmsResult, minIterations) {
+function Benchmark(name, doWarmup, doDeterministic, run, setup, tearDown, latencyResult, minIterations) {
   this.name = name;
   this.doWarmup = doWarmup;
   this.doDeterministic = doDeterministic;
   this.run = run;
   this.Setup = setup ? setup : function() { };
   this.TearDown = tearDown ? tearDown : function() { };
-  this.rmsResult = rmsResult ? rmsResult : null; 
+  this.latencyResult = latencyResult ? latencyResult : null; 
   this.minIterations = minIterations ? minIterations : 32;
 }
 
@@ -189,7 +190,46 @@ BenchmarkSuite.GeometricMeanTime = function(measurements) {
 }
 
 
-// Computes the geometric mean of a set of rms measurements.
+// Computes the average of the worst samples. For example, if percentile is 99, this will report the
+// average of the worst 1% of the samples.
+BenchmarkSuite.AverageAbovePercentile = function(numbers, percentile) {
+  // Don't change the original array.
+  numbers = numbers.slice();
+  
+  // Sort in ascending order.
+  numbers.sort(function(a, b) { return a - b; });
+  
+  // Now the elements we want are at the end. Keep removing them until the array size shrinks too much.
+  // Examples assuming percentile = 99:
+  //
+  // - numbers.length starts at 100: we will remove just the worst entry and then not remove anymore,
+  //   since then numbers.length / originalLength = 0.99.
+  //
+  // - numbers.length starts at 1000: we will remove the ten worst.
+  //
+  // - numbers.length starts at 10: we will remove just the worst.
+  var numbersWeWant = [];
+  var originalLength = numbers.length;
+  while (numbers.length / originalLength > percentile / 100)
+    numbersWeWant.push(numbers.pop());
+  
+  var sum = 0;
+  for (var i = 0; i < numbersWeWant.length; ++i)
+    sum += numbersWeWant[i];
+  
+  var result = sum / numbersWeWant.length;
+  
+  // Do a sanity check.
+  if (numbers.length && result < numbers[numbers.length - 1]) {
+    throw "Sanity check fail: the worst case result is " + result +
+      " but we didn't take into account " + numbers;
+  }
+  
+  return result;
+}
+
+
+// Computes the geometric mean of a set of latency measurements.
 BenchmarkSuite.GeometricMeanLatency = function(measurements) {
   var log = 0;
   var hasLatencyResult = false;
@@ -293,8 +333,10 @@ BenchmarkSuite.prototype.RunSingleBenchmark = function(benchmark, data) {
     // If we've run too few iterations, we continue for another second.
     if (data.runs < benchmark.minIterations) return data;
     var usec = (data.elapsed * 1000) / data.runs;
-    var rms = (benchmark.rmsResult != null) ? benchmark.rmsResult() : 0;
-    this.NotifyStep(new BenchmarkResult(benchmark, usec, rms));
+    var latencySamples = (benchmark.latencyResult != null) ? benchmark.latencyResult() : [0];
+    var percentile = 95;
+    var latency = BenchmarkSuite.AverageAbovePercentile(latencySamples, percentile) * 1000;
+    this.NotifyStep(new BenchmarkResult(benchmark, usec, latency));
     return null;
   }
 }
index 65a49b6..7efb59c 100644 (file)
@@ -1,5 +1,6 @@
 // Portions copyright 2012 Google, Inc.
 // Copyright 2012 Onan Games. All rights reserved.
+// Copyright (C) 2015 Apple Inc. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
@@ -33,11 +34,10 @@ var MandreelBenchmark = new BenchmarkSuite('Mandreel', [16831872, 49852],
                                                            runMandreel,
                                                            setupMandreel,
                                                            tearDownMandreel,
-                                                           RMSMandreel,
+                                                           latencyMandreel,
                                                            4)]);
 
-var mandreelSumSquaredPauses = 0;
-var mandreelSamples = 0;
+var mandreelPauseTimes = [];
 var mandreelSampleTimeStart = 0.0;
 
 function setupMandreel() {
@@ -80,16 +80,15 @@ function runMandreel() {
 function updateMandreelStats(time) {
   var pause = time - mandreelSampleTimeStart;
   mandreelSampleTimeStart = time;
-  mandreelSumSquaredPauses += (pause * pause);
-  mandreelSamples++;
+  mandreelPauseTimes.push(pause);
 }
 
 function startMandreelTimer() {
   mandreelSampleTimeStart = performance.now();
 }
 
-function RMSMandreel() {
-  return Math.round(Math.sqrt(mandreelSumSquaredPauses / mandreelSamples) * 100);
+function latencyMandreel() {
+  return mandreelPauseTimes;
 }
 
 function tearDownMandreel() {
@@ -108,8 +107,7 @@ function tearDownMandreel() {
   heapFloat = null;
   heapDouble = null;
   mandreelAppUsePackAsyncTexture = null;
-  mandreelSumSquaredPauses = 0;
-  mandreelSamples = 0;
+  mandreelPauseTimes = [];
 }
 
 // Mocks for browser functions.
index eff24b3..09fce8b 100644 (file)
@@ -1,4 +1,5 @@
 // Copyright 2009 the V8 project authors. All rights reserved.
+// Copyright (C) 2015 Apple Inc. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
@@ -35,7 +36,7 @@
 
 var Splay = new BenchmarkSuite('Splay', [81491, 2739514], [
   new Benchmark("Splay", true, false, 
-    SplayRun, SplaySetup, SplayTearDown, SplayRMS)
+    SplayRun, SplaySetup, SplayTearDown, SplayLatency)
 ]);
 
 
@@ -68,18 +69,16 @@ function GenerateKey() {
   return Math.random();
 }
 
-var splaySamples = 0;
-var splaySumOfSquaredPauses = 0;
+var splaySamples = [];
 
-function SplayRMS() {
-  return Math.round(Math.sqrt(splaySumOfSquaredPauses / splaySamples) * 10000);
+function SplayLatency() {
+  return splaySamples;
 }
 
 function SplayUpdateStats(time) {
   var pause = time - splaySampleTimeStart;
   splaySampleTimeStart = time;
-  splaySamples++;
-  splaySumOfSquaredPauses += pause * pause;
+  splaySamples.push(pause);
 }
 
 function InsertNewNode() {
@@ -119,8 +118,7 @@ function SplayTearDown() {
   var keys = splayTree.exportKeys();
   splayTree = null;
 
-  splaySamples = 0;
-  splaySumOfSquaredPauses = 0;
+  splaySamples = [];
 
   // Verify that the splay tree has the right size.
   var length = keys.length;
index fa960e6..a18c344 100644 (file)
@@ -54,11 +54,11 @@ JetStream.addReferences({
     "earley-boyer": 2.5795868328750444,
     "regexp-2010": 61.82352941176467,
     "splay": 0.9286376274328075,
-    "splay-latency": 32.399999999999984,
+    "splay-latency": 3.524855736311738,
     "navier-stokes": 9.653846153846146,
     "pdfjs": 88.4166666666666,
     "mandreel": 157.14285714285708,
-    "mandreel-latency": 0.5129999999999998,
+    "mandreel-latency": 1.4940079129942474,
     "gbemu": 135.9999999999998,
     "code-first-load": 2.3249465349251905,
     "box2d": 28.416666666666636,
index 3b45643..c5812fc 100755 (executable)
@@ -1,6 +1,6 @@
 #!/usr/bin/env ruby
 
-# Copyright (C) 2014 Apple Inc. All rights reserved.
+# Copyright (C) 2014, 2015 Apple Inc. All rights reserved.
 #
 # Redistribution and use in source and binary forms, with or without
 # modification, are permitted provided that the following conditions
@@ -26,7 +26,7 @@
 require "pathname"
 require "shellwords"
 
-VERSION = "1.0.1"
+VERSION = "1.1-alpha1"
 DIRECTORY_NAME = "JetStream-#{VERSION}"
 
 raise unless system("rm -rf " + DIRECTORY_NAME)
index 65dab95..287c122 100644 (file)
@@ -537,6 +537,12 @@ public:
         ConcurrentJITLocker locker(m_lock);
         return hasExitSite(locker, site);
     }
+    
+    size_t numberOfExitSites() const
+    {
+        ConcurrentJITLocker locker(m_lock);
+        return m_exitProfile.size();
+    }
 
     DFG::ExitProfile& exitProfile() { return m_exitProfile; }
 
index 40a25ce..42caa3a 100644 (file)
@@ -44,15 +44,9 @@ bool ExitProfile::add(const ConcurrentJITLocker&, const FrequentExitSite& site)
         m_frequentExitSites->append(site);
         return true;
     }
-    
-    // Don't add it if it's already there. This is O(n), but that's OK, because we
-    // know that the total number of places where code exits tends to not be large,
-    // and this code is only used when recompilation is triggered.
-    for (unsigned i = 0; i < m_frequentExitSites->size(); ++i) {
-        if (m_frequentExitSites->at(i) == site)
-            return false;
-    }
-    
+
+    // Always add even if it's a duplicate. The side-effect of doing this is it gives us a
+    // complete count of the number of times we've exited here. 
     m_frequentExitSites->append(site);
     return true;
 }
index cdecbaf..4a46ab6 100644 (file)
@@ -179,6 +179,13 @@ public:
         return hasExitSite(locker, FrequentExitSite(bytecodeIndex, kind));
     }
     
+    size_t size() const
+    {
+        if (!m_frequentExitSites)
+            return 0;
+        return m_frequentExitSites->size();
+    }
+    
 private:
     friend class QueryableExitProfile;
     
index 2c8b0c5..a7d9b0f 100644 (file)
@@ -1273,7 +1273,7 @@ unsigned ByteCodeParser::inliningCost(CallVariant callee, int argumentCountInclu
         dataLog("    Inlining should be possible.\n");
     
     // It might be possible to inline.
-    return codeBlock->instructionCount();
+    return codeBlock->instructionCount() * pow(1 + codeBlock->numberOfExitSites(), Options::exitSitePowerForInlineCost());
 }
 
 template<typename ChecksFunctor>
index fd1682c..a0a3fc3 100644 (file)
@@ -204,12 +204,14 @@ typedef const char* optionString;
     \
     v(unsigned, maximumOptimizationCandidateInstructionCount, 100000, nullptr) \
     \
-    v(unsigned, maximumFunctionForCallInlineCandidateInstructionCount, 180, nullptr) \
-    v(unsigned, maximumFunctionForClosureCallInlineCandidateInstructionCount, 100, nullptr) \
-    v(unsigned, maximumFunctionForConstructInlineCandidateInstructionCount, 100, nullptr) \
+    v(unsigned, maximumFunctionForCallInlineCandidateInstructionCount, 250, nullptr) \
+    v(unsigned, maximumFunctionForClosureCallInlineCandidateInstructionCount, 180, nullptr) \
+    v(unsigned, maximumFunctionForConstructInlineCandidateInstructionCount, 180, nullptr) \
     \
     v(unsigned, maximumFTLCandidateInstructionCount, 20000, nullptr) \
     \
+    v(double, exitSitePowerForInlineCost, 0.5, nullptr) \
+    \
     /* Depth of inline stack, so 1 = no inlining, 2 = one level, etc. */ \
     v(unsigned, maximumInliningDepth, 5, "maximum allowed inlining depth.  Depth of 1 means no inlining") \
     v(unsigned, maximumInliningRecursion, 2, nullptr) \
index d70acf5..d7e412c 100755 (executable)
@@ -2843,7 +2843,7 @@ begin
   
   ARGV.each {
     | vm |
-    if vm =~ /([a-zA-Z0-9_ ]+):/
+    if vm =~ /([a-zA-Z0-9_ .]+):/
       name = $1
       nameKind = :given
       vm = $~.post_match
@@ -2852,7 +2852,7 @@ begin
       nameKind = :auto
     end
     envs = []
-    while vm =~ /([a-zA-Z0-9_]+)=([a-zA-Z0-9_:]+):/
+    while vm =~ /([a-zA-Z0-9_]+)=([a-zA-Z0-9_:.]+):/
       envs << [$1, $2]
       vm = $~.post_match
     end