You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@harmony.apache.org by eg...@apache.org on 2008/02/10 19:33:18 UTC

svn commit: r620305 - in /harmony/enhanced/drlvm/trunk/vm: em/src/DrlEMImpl.cpp em/src/NValueProfileCollector.cpp em/src/NValueProfileCollector.h jitrino/config/em64t/server.emconf jitrino/config/ia32/server.emconf

Author: egor
Date: Sun Feb 10 10:33:15 2008
New Revision: 620305

URL: http://svn.apache.org/viewvc?rev=620305&view=rev
Log:
applying patch:
0003-value-profiling-options-locked-flagged_all-flagged_insert-unsafe.txt
from HARMONY-5396 "[drlvm][em] excess ValueProfiler locking causes degradation on JITted code with profiling"

Enabling the LOCKED strategy (locking only on TNV table restructure). Smoke tests
passed in server mode on my Linux/x86 laptop. Startup time in server mode
improved. Not closing HARMONY-5396 until we make proper code patching.


Modified:
    harmony/enhanced/drlvm/trunk/vm/em/src/DrlEMImpl.cpp
    harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.cpp
    harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.h
    harmony/enhanced/drlvm/trunk/vm/jitrino/config/em64t/server.emconf
    harmony/enhanced/drlvm/trunk/vm/jitrino/config/ia32/server.emconf

Modified: harmony/enhanced/drlvm/trunk/vm/em/src/DrlEMImpl.cpp
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/em/src/DrlEMImpl.cpp?rev=620305&r1=620304&r2=620305&view=diff
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/em/src/DrlEMImpl.cpp (original)
+++ harmony/enhanced/drlvm/trunk/vm/em/src/DrlEMImpl.cpp Sun Feb 10 10:33:15 2008
@@ -16,7 +16,6 @@
  */
 /**
 * @author Mikhail Y. Fursov
-* @version $Revision: 1.1.2.2.4.3 $
 */
 
 #include "DrlEMImpl.h"
@@ -637,22 +636,44 @@
             pc = new EBProfileCollector(this, profilerName, step->jit, ebMode, eThreshold, bThreshold, tbsInitialTimeout, tbsTimeout);
         }
     } else if (profilerType == VALUE_PROFILER_STR) {
+        typedef ValueProfileCollector VPC;
         int vpSteadySize = 4, vpClearSize = 0, vpClearInterval = 0;
         std::string vpalgo = getParam(config, profilerName+".vpalgo");
-        ValueProfileCollector::algotypes vpMode = ValueProfileCollector::TNV_FIRST_N;
+        VPC::algotypes vpMode = VPC::TNV_FIRST_N;
         if (vpalgo == "TNV_DIVIDED") {
-            vpMode = ValueProfileCollector::TNV_DIVIDED;    
+            vpMode = VPC::TNV_DIVIDED;
         } else if (vpalgo != "TNV_FIRST_N") {
             LECHO(10, "EM: unsupported value profiler algotype");
             return NULL;
         }
+
+        std::string strategy = getParam(config, profilerName+".updateStrategy");
+        ProfileUpdateStrategy updateStrategy = UPDATE_LOCKED;
+        if (strategy == "FLAGGED_ALL") {
+            updateStrategy = UPDATE_FLAGGED_ALL;
+        }else if (strategy == "FLAGGED_INSERT") {
+            updateStrategy = UPDATE_FLAGGED_INSERT;
+        }else if (strategy == "UNSAFE") {
+            updateStrategy = UPDATE_UNSAFE;
+        }else if (strategy != "LOCKED") {
+            LECHO(10, "EM: unsupported value profiler updateStrategy");
+            return NULL;
+        }
+
+        if (vpMode != VPC::TNV_FIRST_N && updateStrategy != UPDATE_LOCKED) {
+            LECHO(10, "EM: unsupported value profiler combination of "
+                    "vpalgo and updateStrategy, only TNV_FIRST_N vpalgo "
+                    "curretly sypports other than LOCKED strategy");
+            return NULL;
+        }
+
         bool ok = false;
         vpSteadySize = toNum(getParam(config, profilerName+".vpSteadySize"), &ok);
         if (!ok) {
             LECHO(9, "EM: illegal '{0}' value" << "SteadySize");
             return NULL;
         }
-        if (vpMode == ValueProfileCollector::TNV_DIVIDED) {
+        if (vpMode == VPC::TNV_DIVIDED) {
             vpClearSize = toNum(getParam(config, profilerName+".vpClearSize"), &ok);
             if (!ok) {
                 LECHO(9, "EM: illegal '{0}' value" << "ClearSize");
@@ -664,7 +685,8 @@
                 return NULL;
             }
         }
-        pc = new ValueProfileCollector(this, profilerName, step->jit, vpSteadySize, vpClearSize, vpClearInterval, vpMode);
+        pc = new ValueProfileCollector(this, profilerName, step->jit, vpSteadySize,
+                vpClearSize, vpClearInterval, vpMode, updateStrategy);
     }
     return pc;
 }

Modified: harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.cpp
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.cpp?rev=620305&r1=620304&r2=620305&view=diff
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.cpp (original)
+++ harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.cpp Sun Feb 10 10:33:15 2008
@@ -29,6 +29,9 @@
 #include "cxxlog.h"
 #include <sstream>
 
+#include "port_threadunsafe.h"
+#include "port_atomic.h"
+
 #define LOG_DOMAIN "em"
 
 VPInstructionProfileData* TNVTableManager::createProfileData()
@@ -167,23 +170,45 @@
 void TNVTableFirstNManager::addNewValue(ValueMethodProfile* methProfile,
             VPData* instProfile, ValueT curr_value)
 {
-    // TODO(egor):
-    // 1. options to enable/disable locking
-    // 2. flagged locking
-    methProfile->lockProfile();
+    uint8* updating_ptr = methProfile->getUpdatingStatePtr();
+    if (updateStrategy == UPDATE_FLAGGED_ALL) {
+        // Checking a flag and modifying it atomically must be faster than
+        // locking because it skips simultaneous updates. Faster but sacrifices
+        // profile precision.
+        if (port_atomic_cas8(updating_ptr, 1, 0) != 0) {
+            return;
+        }
+    }
+    UNSAFE_REGION_START
     ValueT* last_value = &(instProfile->last_value);
     uint32* num_times_profiled = &(instProfile->num_times_profiled);
     if (curr_value == *last_value){
+        // We increment the counter safely only with UPDATE_FLAGGED_ALL
         (*num_times_profiled)++;
     } else {
+        if (updateStrategy == UPDATE_LOCKED) {
+            methProfile->lockProfile();
+        }else if (updateStrategy == UPDATE_FLAGGED_INSERT) {
+            if (port_atomic_cas8(updating_ptr, 1, 0) != 0) {
+                return;
+            }
+        }
         struct Simple_TNV_Table* clear_part = instProfile->TNV_clear_part;
         struct Simple_TNV_Table* steady_part = instProfile->TNV_Table;
         flushLastValueCounter(instProfile);
         *num_times_profiled = 1;
         insert(steady_part, clear_part, curr_value, *num_times_profiled);
         *last_value = curr_value;
+        if (updateStrategy == UPDATE_LOCKED) {
+            methProfile->unlockProfile();
+        }else if (updateStrategy == UPDATE_FLAGGED_INSERT) {
+            *updating_ptr = 0;
+        }
+    }
+    UNSAFE_REGION_END
+    if (updateStrategy == UPDATE_FLAGGED_ALL) {
+        *updating_ptr = 0;
     }
-    methProfile->unlockProfile();
 }
 //------------------------------------------------------------------------------
 
@@ -285,16 +310,18 @@
 
 ValueProfileCollector::ValueProfileCollector(EM_PC_Interface* em, const std::string& name, JIT_Handle genJit, 
                                              uint32 _TNV_steady_size, uint32 _TNV_clear_size,
-                                             uint32 _clear_interval, algotypes _TNV_algo_type)
-                                           : ProfileCollector(em, name, EM_PCTYPE_VALUE, genJit)
+                                             uint32 _clear_interval, algotypes _TNV_algo_type,
+                                             ProfileUpdateStrategy update_strategy)
+                                           : ProfileCollector(em, name, EM_PCTYPE_VALUE, genJit),
+                                             updateStrategy(update_strategy)
 {
     hymutex_create(&profilesLock, TM_MUTEX_NESTED);
     if (_TNV_algo_type == TNV_DIVIDED) {
         tnvTableManager = new TNVTableDividedManager
-            (_TNV_steady_size, _TNV_clear_size, _clear_interval);
+            (_TNV_steady_size, _TNV_clear_size, _clear_interval, update_strategy);
     }else if (_TNV_algo_type == TNV_FIRST_N) {
         tnvTableManager = new TNVTableFirstNManager
-            (_TNV_steady_size, _TNV_clear_size, _clear_interval);
+            (_TNV_steady_size, _TNV_clear_size, _clear_interval, update_strategy);
     }
     catName = std::string(LOG_DOMAIN) + ".profiler." + name;
     loggingEnabled =  is_info_enabled(LOG_DOMAIN) ||  is_info_enabled(catName.c_str());
@@ -331,7 +358,7 @@
 //------------------------------------------------------------------------------
 
 ValueMethodProfile::ValueMethodProfile(ValueProfileCollector* pc, Method_Handle mh)
-    : MethodProfile(pc, mh)
+    : MethodProfile(pc, mh), updatingState(0)
 {
     hymutex_create(&lock, TM_MUTEX_DEFAULT);
 }

Modified: harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.h
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.h?rev=620305&r1=620304&r2=620305&view=diff
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.h (original)
+++ harmony/enhanced/drlvm/trunk/vm/em/src/NValueProfileCollector.h Sun Feb 10 10:33:15 2008
@@ -42,16 +42,36 @@
     uint32 frequency;
 };
 
+enum ProfileUpdateStrategy {
+
+    // Lock when restructuring the TNV table (inserting). Trivial counter
+    // increments might not be locked (in current implementation they are not
+    // locked for FirstN table management only)
+    UPDATE_LOCKED,
+
+    // When a new value is added to TNV all simultaneous updates to the table
+    // are skipped
+    UPDATE_FLAGGED_ALL,
+
+    // Only simultaneous inserts to table are skipped, counter updates are
+    // processed unsafely
+    UPDATE_FLAGGED_INSERT,
+
+    // Completely insafe updates of the TNV table (use with care)
+    UPDATE_UNSAFE
+};
+
 class TNVTableManager {
 public:
     typedef struct Simple_TNV_Table TableT;
     typedef POINTER_SIZE_INT ValueT;
     typedef VPInstructionProfileData VPData;
     TNVTableManager(uint32 steady_size, uint32 clear_size,
-            uint32 clear_interval) :
+            uint32 clear_interval, ProfileUpdateStrategy update_strategy) :
         steadySize(steady_size),
         clearSize(clear_size),
-        clearInterval(clear_interval)
+        clearInterval(clear_interval),
+        updateStrategy(update_strategy)
     {}
 
     VPInstructionProfileData* createProfileData();
@@ -83,14 +103,15 @@
             ValueT value_to_insert, uint32 times_met) = 0;
 
     const uint32 steadySize, clearSize, clearInterval;
+    const ProfileUpdateStrategy updateStrategy;
 };
 
 class TNVTableDividedManager : public TNVTableManager {
 public:
     // c-tor
     TNVTableDividedManager(uint32 steady_size, uint32 clear_size,
-            uint32 clear_interval) :
-        TNVTableManager(steady_size, clear_size, clear_interval)
+            uint32 clear_interval, ProfileUpdateStrategy us) :
+        TNVTableManager(steady_size, clear_size, clear_interval, us)
     {}
 
     virtual void addNewValue(ValueMethodProfile* methProfile,
@@ -105,8 +126,8 @@
 public:
     // c-tor
     TNVTableFirstNManager(uint32 steady_size, uint32 clear_size,
-            uint32 clear_interval) :
-        TNVTableManager(steady_size, clear_size, clear_interval)
+            uint32 clear_interval, ProfileUpdateStrategy us) :
+        TNVTableManager(steady_size, clear_size, clear_interval, us)
     {}
 
     virtual void addNewValue(ValueMethodProfile* methProfile,
@@ -120,11 +141,15 @@
 
 class ValueProfileCollector : public ProfileCollector {
 public:
-    enum algotypes {TNV_DIVIDED, TNV_FIRST_N};
+    enum algotypes {
+        TNV_DIVIDED,
+        TNV_FIRST_N
+    };
 
     ValueProfileCollector(EM_PC_Interface* em, const std::string& name, JIT_Handle genJit,
                                                 uint32 _TNV_steady_size, uint32 _TNV_clear_size,
-                                                uint32 _clear_interval, algotypes _TNV_algo_type);
+                                                uint32 _clear_interval, algotypes _TNV_algo_type,
+                                                ProfileUpdateStrategy update_strategy);
 
     virtual TbsEMClient* getTbsEmClient() const {return (NULL);}
     virtual ~ValueProfileCollector();
@@ -139,6 +164,7 @@
     ValueProfilesMap profilesByMethod;
     mutable hymutex_t profilesLock;
     TNVTableManager* tnvTableManager;
+    ProfileUpdateStrategy updateStrategy;
 };
 
 class VPInstructionProfileData
@@ -161,8 +187,6 @@
 typedef std::map<uint32, VPInstructionProfileData*> VPDataMap;
 class ValueMethodProfile : public MethodProfile {
 public:
-    VPDataMap ValueMap; // TODO: make it private
-public:
     ValueMethodProfile(ValueProfileCollector* pc, Method_Handle mh);
     ~ValueMethodProfile();
     void lockProfile() {hymutex_lock(&lock);}
@@ -170,10 +194,22 @@
     void dumpValues(std::ostream& os);
     void addNewValue(uint32 instructionKey, POINTER_SIZE_INT valueToAdd);
     POINTER_SIZE_INT getResult(uint32 instructionKey);
+
+    // UpatingState is used to implement UPDATE_FLAGGED_* strategies.
+    //     (updatingState == 1) when method profile is being updated to skip
+    //         concurrent modifications.
+    //     (updatingState == 0) when profile is open for modifications.
+    uint8* getUpdatingStatePtr() { return &updatingState; }
 private:
     ValueProfileCollector* getVPC() const;
 
+    friend class ValueProfileCollector;
+    VPDataMap ValueMap;
+
+    // The lock and the atomically modified updatingState flag operate per
+    // method to allow simultaneous updates for distinct methods.
     hymutex_t lock;
+    uint8 updatingState;
 };
 
 POINTER_SIZE_INT value_profiler_get_top_value (Method_Profile_Handle mph, uint32 instructionKey);

Modified: harmony/enhanced/drlvm/trunk/vm/jitrino/config/em64t/server.emconf
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/jitrino/config/em64t/server.emconf?rev=620305&r1=620304&r2=620305&view=diff
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/jitrino/config/em64t/server.emconf (original)
+++ harmony/enhanced/drlvm/trunk/vm/jitrino/config/em64t/server.emconf Sun Feb 10 10:33:15 2008
@@ -43,6 +43,7 @@
 
 VALUE_PROF.profilerType=VALUE_PROFILER
 VALUE_PROF.vpalgo=TNV_FIRST_N
+VALUE_PROF.updateStrategy=LOCKED
 VALUE_PROF.vpSteadySize=4
 SD1_OPT.genProfile=EDGE_PROF,VALUE_PROF
 SD2_OPT.useProfile=EDGE_PROF,VALUE_PROF

Modified: harmony/enhanced/drlvm/trunk/vm/jitrino/config/ia32/server.emconf
URL: http://svn.apache.org/viewvc/harmony/enhanced/drlvm/trunk/vm/jitrino/config/ia32/server.emconf?rev=620305&r1=620304&r2=620305&view=diff
==============================================================================
--- harmony/enhanced/drlvm/trunk/vm/jitrino/config/ia32/server.emconf (original)
+++ harmony/enhanced/drlvm/trunk/vm/jitrino/config/ia32/server.emconf Sun Feb 10 10:33:15 2008
@@ -43,6 +43,7 @@
 
 VALUE_PROF.profilerType=VALUE_PROFILER
 VALUE_PROF.vpalgo=TNV_FIRST_N
+VALUE_PROF.updateStrategy=LOCKED
 VALUE_PROF.vpSteadySize=4
 SD1_OPT.genProfile=EDGE_PROF,VALUE_PROF
 SD2_OPT.useProfile=EDGE_PROF,VALUE_PROF