From 06ee8cd72558018e701f8173e4ed0a5e55be2730 Mon Sep 17 00:00:00 2001
From: Andy Ayers <andya@microsoft.com>
Date: Tue, 12 Jul 2016 16:12:46 -0700
Subject: [PATCH] Inliner: Update data collection by targeting a single inline

The inliner currently will record detailed data about the last successful
inline performed (given a build with DEBUG or INLINE_DATA defined).
However, for purposes of inline profitability analysis we might be more
interested in the data from an earlier inline.

This change creates a mechanism where the replay log can flag one inline
per method as the target of data collection. The inliner checks for this
attribute during replay and captures that inline's data.
---
 src/jit/inline.cpp       | 19 +++++++++++++++++--
 src/jit/inline.h         | 12 ++++++++++++
 src/jit/inlinepolicy.cpp | 22 +++++++++++++++++++---
 3 files changed, 48 insertions(+), 5 deletions(-)

diff --git a/src/jit/inline.cpp b/src/jit/inline.cpp
index 146ef0585fc2..cca67570b101 100644
--- a/src/jit/inline.cpp
+++ b/src/jit/inline.cpp
@@ -955,8 +955,23 @@ void InlineStrategy::NoteOutcome(InlineContext* context)
 
 #if defined(DEBUG) || defined(INLINE_DATA)
 
-        m_LastContext = context;
-        m_LastSuccessfulPolicy = context->m_Policy;
+        // Keep track of the inline targeted for data collection or,
+        // if we don't have one (yet), the last successful inline.
+        bool updateLast =
+            (m_LastSuccessfulPolicy == nullptr) ||
+            !m_LastSuccessfulPolicy->IsDataCollectionTarget();
+
+        if (updateLast)
+        {
+            m_LastContext = context;
+            m_LastSuccessfulPolicy = context->m_Policy;
+        }
+        else
+        {
+            // We only expect one inline to be a data collection
+            // target.
+            assert(!context->m_Policy->IsDataCollectionTarget());
+        }
 
 #endif // defined(DEBUG) || defined(INLINE_DATA)
 
diff --git a/src/jit/inline.h b/src/jit/inline.h
index effe4219031d..0e8739d7fc46 100644
--- a/src/jit/inline.h
+++ b/src/jit/inline.h
@@ -256,6 +256,8 @@ class InlinePolicy
     virtual void DumpData(FILE* file) const { }
     // Detailed data name dump
     virtual void DumpSchema(FILE* file) const { }
+    // True if this is the inline targeted by data collection
+    bool IsDataCollectionTarget() { return m_IsDataCollectionTarget; }
 
 #endif // defined(DEBUG) || defined(INLINE_DATA)
 
@@ -265,6 +267,10 @@ class InlinePolicy
         : m_Decision(InlineDecision::UNDECIDED)
         , m_Observation(InlineObservation::CALLEE_UNUSED_INITIAL)
         , m_IsPrejitRoot(isPrejitRoot)
+#if defined(DEBUG) || defined(INLINE_DATA)
+        , m_IsDataCollectionTarget(false)
+#endif // defined(DEBUG) || defined(INLINE_DATA)
+
     {
         // empty
     }
@@ -280,6 +286,12 @@ class InlinePolicy
     InlineDecision    m_Decision;
     InlineObservation m_Observation;
     bool              m_IsPrejitRoot;
+
+#if defined(DEBUG) || defined(INLINE_DATA)
+
+    bool              m_IsDataCollectionTarget;
+
+#endif // defined(DEBUG) || defined(INLINE_DATA)
 };
 
 // InlineResult summarizes what is known about the viability of a
diff --git a/src/jit/inlinepolicy.cpp b/src/jit/inlinepolicy.cpp
index d60d9bff0505..eee337eeadf7 100644
--- a/src/jit/inlinepolicy.cpp
+++ b/src/jit/inlinepolicy.cpp
@@ -2038,7 +2038,7 @@ void ModelPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo)
         // positive be better and negative worse.
         double perCallBenefit = -((double) m_PerCallInstructionEstimate / (double) m_ModelCodeSizeEstimate);
 
-        // Now estimate the local call frequency.  
+        // Now estimate the local call frequency.
         //
         // Todo: use IBC data, or a better local profile estimate, or
         // try and incorporate this into the model. For instance if we
@@ -2072,8 +2072,8 @@ void ModelPolicy::DetermineProfitability(CORINFO_METHOD_INFO* methodInfo)
         // is our benefit figure of merit.
         double benefit = callSiteWeight * perCallBenefit;
 
-        // Compare this to the threshold, and inline if greater.  
-        // 
+        // Compare this to the threshold, and inline if greater.
+        //
         // The threshold is interpretable as a size/speed tradeoff:
         // the value of 0.2 below indicates we'll allow inlines that
         // grow code by as many as 5 bytes to save 1 instruction
@@ -2583,6 +2583,22 @@ bool ReplayPolicy::FindInline(unsigned token, unsigned hash, unsigned offset)
 
         // We're good!
         foundInline = true;
+
+        // Check for a data collection marker. This does not affect
+        // matching...
+
+        // Get next line
+        if (fgets(buffer, sizeof(buffer), s_ReplayFile) != nullptr)
+        {
+            unsigned collectData = 0;
+            count = sscanf(buffer, " <CollectData>%u</CollectData> ", &collectData);
+
+            if (count == 1)
+            {
+                m_IsDataCollectionTarget = (collectData == 1);
+            }
+        }
+
         break;
     }