Replaced ADEvent's / ExecData_t's method of labeling events as deletable by the garbage collection with a reference count strategy where events are only trimmed when the reference count is 0

	This fixes a bug found where common parents of host events that have GPU events were getting prematurely trimmed
	Added a unit test for the above case
Implemented ADEvent::clear

Author: giltirn

include/chimbuko/ad/ADEvent.hpp

@@ -252,7 +252,7 @@ namespace chimbuko {
     std::pair<CallListIterator_t, CallListIterator_t> getCallWindowStartEnd(const eventID &event_id, const int win_size) const override;
 
     /**
-     * @brief clear
+     * @brief Clear all data members
      *
      */
     void clear();
@@ -405,11 +405,6 @@ namespace chimbuko {
      */
     std::unordered_map<unsigned long, CallListIterator_t> m_unmatchedCorrelationID;
 
-    /**
-     * @brief Map of event index to the number of unmatched correlation IDs
-     */
-    std::unordered_map<eventID,size_t> m_unmatchedCorrelationID_count;
-
     /**
      * @brief verbose
      *

include/chimbuko/ad/ExecData.hpp

@@ -594,13 +594,22 @@ class ExecData_t {
     /**
      * @brief Determine whether the event can be deleted by the garbage collection at the end of the io step
      */
-    bool can_delete() const{ return m_can_delete;}
+    inline bool can_delete() const{ return m_references == 0;}
 
     /**
-     * @brief Set whether the event can be deleted by the garbage collection at the end of the io step (default true)
-     */
-    void can_delete(const bool v){ m_can_delete = v; }
+     * @brief Increment the external reference counter, preventing object deletion
+    */
+    inline void register_reference(){ ++m_references; }
+
+    /**
+     * @brief Decrement the external reference counter, allowing object deletion if 0
+    */
+    void deregister_reference();
 
+    /**
+     * @brief Get the number of external references registered
+     */
+    inline unsigned long reference_count() const{ return m_references; }
 
     /**
      * @brief Set the partner event linked by a GPU correlation ID
@@ -645,7 +654,7 @@ class ExecData_t {
     unsigned long m_n_messages;          /**< the number of messages */
     std::deque<CommData_t> m_messages;  /**< a vector of all messages */
     std::deque<CounterData_t> m_counters; /**< a vector of all counters */
-    bool m_can_delete; /**< Flag indicating that the event is deletable by the garbage collection */
+    unsigned long m_references; /**< track number of external references to object. When 0 the object can be deleted */
     std::vector<eventID> m_gpu_correlation_id_partner;  /**< The event ids partner events linked by a correlation ID, either the launching CPU event or the GPU kernel event */
 };
 

src/ad/ADEvent.cpp

@@ -15,10 +15,20 @@ ADEvent::ADEvent(bool verbose)
 }
 
 ADEvent::~ADEvent() {
-  clear();
 }
 
 void ADEvent::clear() {
+  m_eidx_func_entry = -1;
+  m_eidx_func_exit = -1;
+  m_eidx_comm_send = -1;
+  m_eidx_comm_recv = -1;
+  m_commStack.clear();
+  m_counterStack.clear();
+  m_callStack.clear();
+  m_callList.clear();
+  m_execDataMap.clear();
+  m_callIDMap.clear();
+  m_unmatchedCorrelationID.clear();
 }
 
 EventError ADEvent::addEvent(const Event_t& event) {
@@ -32,7 +42,8 @@ EventError ADEvent::addEvent(const Event_t& event) {
 }
 
 void ADEvent::stackProtectGC(CallListIterator_t it){
-  it->can_delete(false);
+  verboseStream << "ADEvent::stackProtectGC incrementing register count of " << it->get_id().toString() << " to " << it->reference_count()+1 << std::endl;
+  it->register_reference();
 
   eventID parent = it->get_parent();
   while(parent != eventID::root()){
@@ -43,18 +54,15 @@ void ADEvent::stackProtectGC(CallListIterator_t it){
       recoverable_error("Could not find parent " + parent.toString() + " in call list due to : " + e.what());
       break;
     }
-
-    pit->can_delete(false);
+    verboseStream << "ADEvent::stackProtectGC incrementing register count of " << pit->get_id().toString() << " to " << pit->reference_count()+1 << std::endl;
+    pit->register_reference();
     parent = pit->get_parent();
   }
 }
 
 void ADEvent::stackUnProtectGC(CallListIterator_t it){
-  //Check if has unmatched correlation iD
-  auto umit = m_unmatchedCorrelationID_count.find(it->get_id());
-  if(umit == m_unmatchedCorrelationID_count.end() || umit->second == 0)
-    it->can_delete(true);
-  else return; //stop here, the stack will be needed
+  verboseStream << "ADEvent::stackUnProtectGC decrementing register count of " << it->get_id().toString() << " to " << it->reference_count()-1 << std::endl;
+  it->deregister_reference();
 
   eventID parent = it->get_parent();
   while(parent != eventID::root()){
@@ -65,12 +73,8 @@ void ADEvent::stackUnProtectGC(CallListIterator_t it){
       recoverable_error("Could not find parent " + parent.toString() + " in call list due to : " + e.what());
       break;
     }
-
-    umit = m_unmatchedCorrelationID_count.find(pit->get_id());
-    if(umit == m_unmatchedCorrelationID_count.end() || umit->second == 0)
-      pit->can_delete(true);
-    else break; //stop here, the stack will be needed
-
+    verboseStream << "ADEvent::stackUnProtectGC decrementing register count of " << pit->get_id().toString() << " to " << pit->reference_count()-1 << std::endl;
+    pit->deregister_reference();
     parent = pit->get_parent();
   }
 }
@@ -85,29 +89,27 @@ void ADEvent::checkAndMatchCorrelationID(CallListIterator_t it){
   for(auto const &c : it->get_counters()){
     if(c.get_countername() == "Correlation ID"){
       unsigned long cid = c.get_value();
+      eventID current_event_id = it->get_id();
 
       //Does a partner already exist?
       auto m = m_unmatchedCorrelationID.find(cid);
       if(m != m_unmatchedCorrelationID.end()){
-	eventID current_event_id = it->get_id();
 	eventID partner_event_id = m->second->get_id();
+
+	//Check partner event hasn't been accidentally deleted (thus invalidating its iterator)
+	if(m_callIDMap.find(partner_event_id) == m_callIDMap.end()) fatal_error("Correlation ID partner event is not in the call list!");
+
 	it->set_GPU_correlationID_partner(partner_event_id);
 	m->second->set_GPU_correlationID_partner(current_event_id);
 
-	size_t rem = --m_unmatchedCorrelationID_count[partner_event_id];
-	if(rem == 0){
-	  m_unmatchedCorrelationID_count.erase(partner_event_id); //no need to keep this around now
-	  stackUnProtectGC(m->second);
-	}
+	verboseStream << "Current event " << current_event_id.toString() << " is partnered with previous unmatched event " << partner_event_id.toString() << " with correlation ID " << cid << std::endl;
+	stackUnProtectGC(m->second);
 	m_unmatchedCorrelationID.erase(cid); //remove now-matched correlation ID
-
-	verboseStream << "Found partner event " << current_event_id.toString() << " to previous unmatched event " << partner_event_id.toString() << " with correlation ID " << cid << std::endl;
       }else{
 	//Ensure the event and it's parental line can't be deleted and put it in the map of unmatched events
+	verboseStream << "Found as-yet unpartnered event " << current_event_id.toString() << " with correlation ID " << cid << std::endl;
 	stackProtectGC(it);
 	m_unmatchedCorrelationID[cid] = it;
-	++m_unmatchedCorrelationID_count[it->get_id()];
-	verboseStream << "Found as-yet unpartnered event with correlation ID " << cid << std::endl;
       }
       n_cid++;
     }

src/ad/ExecData.cpp

@@ -10,7 +10,7 @@ ExecData_t::ExecData_t()
   : m_pid(0), m_rid(0), m_tid(0), m_fid(0),
     m_entry(0), m_exit(0), m_runtime(0), m_exclusive(0),
     m_n_children(0), m_n_messages(0),
-    m_label(0), m_can_delete(true), m_gpu_correlation_id_partner(0), m_score(-1){}
+    m_label(0), m_gpu_correlation_id_partner(0), m_score(-1), m_references(0){}
 
 ExecData_t::ExecData_t(const Event_t& ev) : ExecData_t()
 {
@@ -39,6 +39,11 @@ ExecData_t::ExecData_t(const eventID &id, unsigned long pid, unsigned long rid,
 
 ExecData_t::~ExecData_t() {}
 
+void ExecData_t::deregister_reference(){ 
+  if(m_references == 0) fatal_error("Attempting to deregister and event whose external reference count is 0!");
+  --m_references; 
+}
+
 void ExecData_t::update_exit(unsigned long exit){
   m_exit = exit;
   m_runtime = m_exit - m_entry;

test/unit_tests/ad/ADAnomalyProvenance.cpp

@@ -359,6 +359,8 @@ TEST(TestADAnomalyProvenance, gracefullyFailsIfCorrelationIDissues){
 
   {
     std::cout <<  "Testing failure due to missing parent event" << std::endl;
+    event_man.clear();
+    ASSERT_EQ(event_man.getCallListSize(),0);
 
     //Have a host correlation ID but not a device one
     int corridx4 = 4444;
@@ -369,13 +371,20 @@ TEST(TestADAnomalyProvenance, gracefullyFailsIfCorrelationIDissues){
     exec_cpu.add_counter(createCounterData_t(0,1, 0, corrid_cid,  corridx4, 1000, "Correlation ID"));
 
     CallListIterator_t exec_cpu_it = event_man.addCall(exec_cpu);
+    CallListIterator_t exec_gpu_it = event_man.addCall(exec_gpu);
 
-    //Force the trimming out of the cpu event
-    exec_cpu_it->can_delete(true);
+    //Both events should be trimmable
+    ASSERT_EQ(exec_cpu_it->reference_count(),0);
+    ASSERT_EQ(exec_gpu_it->reference_count(),0);
+    
+    //Trim out the parent
+    exec_gpu_it->register_reference(); //protect GPU event
     delete event_man.trimCallList();
-
-    CallListIterator_t exec_gpu_it = event_man.addCall(exec_gpu);
 
+    ASSERT_EQ(event_man.getCallListSize(),1);
+    std::cout << "Trimmed out event " << exec_cpu.get_id().toString() << std::endl;
+    	
+    std::cout << "Creating ADAnomalyProvenance" << std::endl;
     ADAnomalyProvenance prov_gpu(*exec_gpu_it,
 				 event_man,
 				 param,

test/unit_tests/ad/ADEvent.cpp

@@ -454,7 +454,10 @@ TEST(ADEvent, trimsCallListCorrectlyWithGCFlag){
 
   //Add 2 events but mark 1 to be undeletable
   event_man.addCall(c1);
-  c2.can_delete(false);
+  c2.register_reference();
+  EXPECT_EQ(c2.can_delete(), false);
+  EXPECT_EQ(c2.reference_count(), 1);
+
   event_man.addCall(c2);
 
   auto const* calls_p_r_t_ptr = getElemPRT(pid,rid,tid,event_man.getCallListMap());
@@ -509,10 +512,30 @@ TEST(ADEvent, matchesEventsByCorrelationID){
   ExecData_t c_cpu_p2 = createFuncExecData_t(pid, rid, tid_cpu, 7, "cpu_launch_kernel_grandparent", 0, 300); //0-300
   bindParentChild(c_cpu_p2, c_cpu_p1);
 
+  std::cout << "c_cpu_p2 : " << c_cpu_p2.get_id().toString() << std::endl;
+  std::cout << "c_cpu_p1 : " << c_cpu_p1.get_id().toString() << std::endl;
+  std::cout << "c_cpu : " << c_cpu.get_id().toString() << std::endl;
+
+
+  auto c_cpu_p2_it = event_man.addCall(c_cpu_p2);
 
-  event_man.addCall(c_cpu_p2);
-  event_man.addCall(c_cpu_p1);
-  event_man.addCall(c_cpu);
+  //c_cpu should not be locked yet
+  ASSERT_EQ(c_cpu_p2_it->reference_count(), 0);
+
+  auto c_cpu_p1_it = event_man.addCall(c_cpu_p1);
+  
+  //This should lock both c_cpu_p2 and c_cpu_p1
+  ASSERT_EQ(c_cpu_p2_it->reference_count(), 1);
+  ASSERT_EQ(c_cpu_p1_it->reference_count(), 1);
+
+  auto c_cpu_it = event_man.addCall(c_cpu);
+
+  //c_cpu should be doubly locked
+  ASSERT_EQ(c_cpu_it->reference_count(), 2);
+  //c_cpu_p1 should be triply locked
+  ASSERT_EQ(c_cpu_p1_it->reference_count(), 3);
+  //c_cpu_p2 should be triply locked
+  ASSERT_EQ(c_cpu_p2_it->reference_count(), 3);
 
   //Ensure the correlation IDs got picked up
   EXPECT_EQ( event_man.getUnmatchCorrelationIDevents().size(), 3 );
@@ -589,6 +612,66 @@ TEST(ADEvent, matchesEventsByCorrelationID){
 
 
 
+TEST(ADEvent, testCoridMatchChainUnlock){
+  //Ensure that for a tree with multiple GPU events and common parents, when unlocking the tree for the first event
+  //that we don't allow the common parents to get unlocked before the second event is matched
+
+  ADEvent event_man;
+  int pid = 1;
+  int rid = 2;
+  int tid_cpu = 3;
+  int tid_gpu = 4;
+  int counter_id = 13; //index of "Correlation ID" counter
+
+  //First CPU event
+  ExecData_t c_cpu_1 = createFuncExecData_t(pid, rid, tid_cpu, 4, "cpu_launch_kernel_1", 100, 200); //100-300
+  c_cpu_1.add_counter(createCounterData_t(pid,rid,tid_cpu,counter_id, 1995, 100, "Correlation ID"));
+
+  ExecData_t c_gpu1 = createFuncExecData_t(pid, rid, tid_gpu, 6, "gpu_kernel1", 400, 100); //400-500
+  c_gpu1.add_counter(createCounterData_t(pid,rid,tid_gpu,counter_id, 1995, 500, "Correlation ID"));
+
+  //Second CPU event
+  ExecData_t c_cpu_2 = createFuncExecData_t(pid, rid, tid_cpu, 5, "cpu_launch_kernel_2", 150, 250); //150-250
+  c_cpu_2.add_counter(createCounterData_t(pid,rid,tid_cpu,counter_id, 2020, 150, "Correlation ID"));
+
+  ExecData_t c_gpu2 = createFuncExecData_t(pid, rid, tid_gpu, 7, "gpu_kernel2", 500, 100); //500-600
+  c_gpu2.add_counter(createCounterData_t(pid,rid,tid_gpu,counter_id, 2020, 600, "Correlation ID"));
+
+  //Common parent event
+  ExecData_t c_cpu_p1 = createFuncExecData_t(pid, rid, tid_cpu, 8, "cpu_launch_kernel_parent", 50, 500); //50-550
+  bindParentChild(c_cpu_p1, c_cpu_1);
+  bindParentChild(c_cpu_p1, c_cpu_2);
+
+  auto c_cpu_p1_it =  event_man.addCall(c_cpu_p1);
+  auto c_cpu_1_it = event_man.addCall(c_cpu_1);
+  auto c_cpu_2_it = event_man.addCall(c_cpu_2);
+
+  //Ensure the correlation IDs got picked up
+  EXPECT_EQ( event_man.getUnmatchCorrelationIDevents().size(), 2 );
+
+  //Both the CPU and parent event should be locked
+  EXPECT_EQ( c_cpu_p1_it->can_delete(), false );
+  EXPECT_EQ( c_cpu_1_it->can_delete(), false );
+  EXPECT_EQ( c_cpu_2_it->can_delete(), false );
+
+
+  //Add the first GPU event
+  auto c_gpu1_it = event_man.addCall(c_gpu1);
+
+  //Check correlation ID was matched
+  EXPECT_EQ( event_man.getUnmatchCorrelationIDevents().size(), 1 );
+
+  //Check both gpu1 and cpu1 are unlocked
+  EXPECT_EQ( c_cpu_1_it->can_delete(), true );
+  EXPECT_EQ( c_gpu1_it->can_delete(), true );
+
+  //Check parent is *not* unlocked
+  EXPECT_EQ( c_cpu_p1_it->can_delete(), false ); 
+}
+
+
+
+
 
 TEST(ADEvent, testIteratorWindowDetermination){
   ADEvent event_man;