Bug #1811: cross section reduction accepts a callback now 46/3846/8
authorraghavendrak <raghavendra066@gmail.com>
Tue, 13 Mar 2018 19:22:04 +0000 (14:22 -0500)
committerSam White <white67@illinois.edu>
Sat, 11 Aug 2018 15:33:24 +0000 (10:33 -0500)
Previously setReductionClient was required for call back to work in
cross section reduction. With this patch there is no longer that
requirement.

Change-Id: If823a73f1a15f6210dcd99e054912b7242330ef1

doc/charm++/sections.tex
examples/charm++/hello/Makefile
examples/charm++/hello/xarraySection/Makefile
examples/charm++/hello/xarraySection/hello.C
src/ck-core/XArraySectionReducer.h
src/ck-core/ckmulticast.C

index 59314c9367d80411aa2354c571b379081ff37da7..29d718cb93741bd9c7286fb15bceba2be2bb7ec6 100644 (file)
@@ -239,9 +239,6 @@ array sections with the following restrictions.
 
 \item The elements within each array must be enumerated explicitly.
 
-\item No existing modules currently support delegation of cross
-  section proxies. Therefore reductions are not currently supported.
-
 \end{itemize}
 
 Note: cross section logic also works for groups with analogous characteristics.
@@ -285,9 +282,11 @@ The resulting cross section proxy, as in the example \uw{arrayLowProxy},
 can then be used for multicasts in the same way as a normal array
 section.
 
-Note: For simplicity the example has all arrays and sections of uniform
+Note: For simplicity the above example has all arrays and sections of uniform
 size.  The size of each array and the number of elements in each array
 within a section can all be set independently.
+For a more concrete example on how to use cross array section reduction,
+please refer to: \examplerefdir{hello/xarraySection}.
 
 
 
index 0a88badb08d992a4ff7c73ab1d391baa4edf58ad..018069b407c0af307c60c20b4c11b9200e528a1e 100644 (file)
@@ -10,6 +10,7 @@ DIRS = \
   4darray \
   nodegroup \
   1darray \
+  xarraySection \
 
 all: $(foreach i,$(DIRS),build-$i)
 
index 9a5869e083f36390d879e72e316815fb2b51f3f8..bea62a8ce8c8ac84c167d314d46d981d25d3c12a 100644 (file)
@@ -48,7 +48,7 @@ again:
 
 test: all
        @echo "########################################################################################"
-       $(call run, $(EXECFLAGS) $(TARGET) $(ARGS))
+       $(call run, $(EXECFLAGS) ./$(TARGET) $(ARGS))
 
 ####### Pattern rules
 # Rule to generate dependency information for C++ source files
index 32b9fb4cb4422e5f0807e6f7613559023298296d..20c600bd431069685248efa02b5792b0b3e0cf6e 100644 (file)
@@ -18,9 +18,6 @@ class Main : public CBase_Main
 public:
   Main(CkArgMsg* m): numArrays(2), numElements(5), sectionSize(numElements), maxIter(3), numIter(0)
   {
-    CkAbort("This test does not currently work and is not expected to work" \
-            " until support for cross array reductions is added to the multicast manager.");
-
     // Save a proxy to main for use as a reduction target
     mainProxy = thisProxy;
 
index 764d1a6e5f5983a36224e9a42553065d5663b2cd..9961e09f1b1271ccde4c91fe3c97b94aeafc71a5 100644 (file)
@@ -20,8 +20,7 @@ class XArraySectionReducer
     public:
         ///
         XArraySectionReducer(int _numSubSections, CkCallback *_finalCB)
-            : numSubSections(_numSubSections), finalCB(_finalCB), numReceived(0),
-              msgList(numSubSections, nullptr)
+            : numSubSections(_numSubSections), finalCB(_finalCB)
         {
             CkAssert(numSubSections > 0);
         }
@@ -35,37 +34,43 @@ class XArraySectionReducer
         /// Each subsection reduction message needs to be passed in here
         void acceptSectionContribution(CkReductionMsg *msg)
         {
-            msgList[numReceived++] = msg;
-            if (numReceived >= numSubSections)
-                finalReducer();
+            msgMap[msg->redNo].push_back(msg);
+            numReceivedMap[msg->redNo]++;
+            if (numReceivedMap[msg->redNo] >= numSubSections)
+              finalReducer(msg->redNo);
         }
 
     private:
-        /// Triggered after all subsections have completed their reductions
-        void finalReducer()
+        /// Triggered after all subsections have completed their reductions for a particular redNo
+        void finalReducer(int redNo)
         {
             // Get a handle on the reduction function for this message
-            CkReduction::reducerFn f = CkReduction::reducerTable()[ msgList[0]->reducer ].fn;
+            CkReduction::reducerFn f = CkReduction::reducerTable()[ msgMap[redNo][0]->reducer ].fn;
             // Perform an extra reduction step on all the subsection reduction msgs
-            CkReductionMsg *finalMsg = (*f)(numSubSections, msgList.data());
+            CkReductionMsg *finalMsg = (*f)(numSubSections, msgMap[redNo].data());
             // Send the final reduced msg to the client
-            finalCB->send(finalMsg);
+            if (finalCB == nullptr)
+                msgMap[redNo][0]->callback.send(finalMsg);
+            else
+                finalCB->send(finalMsg);
             // Delete the subsection redn msgs, accounting for any msg reuse
             for (int i=0; i < numSubSections; i++)
-                if (msgList[i] != finalMsg) delete msgList[i];
-            // Reset the msg list and counters in preparation for the next redn
-            memset( msgList.data(), 0, numSubSections*sizeof(CkReductionMsg*) );
-            numReceived = 0;
+                if (msgMap[redNo][i] != finalMsg) delete msgMap[redNo][i];
+            // Reset the msg list and counters for the corresponding redNo
+            memset( msgMap[redNo].data(), 0, numSubSections*sizeof(CkReductionMsg*) );
+            numReceivedMap[redNo] = 0;
         }
 
         // The number of subsection redn msgs to expect
         const int numSubSections;
         // The final client callback after all redn are done
         const CkCallback *finalCB;
-        // Counter to track when all subsection redns are complete
-        int numReceived;
-        // List of subsection redn msgs
-        std::vector<CkReductionMsg *> msgList;
+        // A map of counters indexed using redNo to track when all the subsection redns are complete
+        // Since a single instance of this class is used to stitch the reductions across arrays,
+        // multiple callbacks to a particular cross section array are tagged using their redNo
+        std::map<int, int> numReceivedMap;
+        // A map storing a list of subsection redn msgs indexed using redNo
+        std::map<int, std::vector<CkReductionMsg *> > msgMap;
 };
 
 
index b73db09e325f2d390eec6151bcbe1bdf09c9a43d..9003fa7d2fcba870171d1fb20438ba817a072e5a 100644 (file)
@@ -452,12 +452,33 @@ void CkMulticastMgr::initDelegateMgr(CProxy *cproxy, int opts)
 
   CProxySection_ArrayBase *proxy = (CProxySection_ArrayBase *)cproxy;
   int numSubSections = proxy->ckGetNumSubSections();
+  CkCallback *sectionCB;
+  // If its a cross-array section,
+  if (numSubSections > 1)
+  {
+      /** @warning: Each instantiation is a mem leak! :o
+       * The class is trivially small, but there's one instantiation for each
+       * section delegated to CkMulticast. The objects need to live as long as
+       * their section exists and is used. The idea of 'destroying' an array
+       * section is still academic, and hence no effort has been made to charge
+       * some 'owner' entity with the task of deleting this object.
+       *
+       * Reimplementing delegated x-array reductions will make this consideration moot
+       */
+      // Configure the final cross-section reducer
+      ck::impl::XArraySectionReducer *red =
+          new ck::impl::XArraySectionReducer(numSubSections, nullptr);
+      // Configure the subsection callback to deposit with the final reducer
+      sectionCB = new CkCallback(ck::impl::processSectionContribution, red);
+  }
   for (int i=0; i<numSubSections; i++)
   {
       CkArrayID aid = proxy->ckGetArrayIDn(i);
       mCastEntry *entry = new mCastEntry(aid);
       CkSectionID *sid = &( proxy->ckGetSectionID(i) );
       const CkArrayIndex *al = proxy->ckGetArrayElements(i);
+      if (numSubSections > 1)
+          entry->red.storedCallback = sectionCB;
       prepareCookie(entry, *sid, al, proxy->ckGetNumElements(i), aid);
       initCookie(sid->_cookie);
   }
@@ -1161,20 +1182,20 @@ void CkMulticastMgr::setReductionClient(CProxySection_ArrayBase &proxy, CkCallba
   // If its a cross-array section,
   if (numSubSections > 1)
   {
-      /** @warning: Each instantiation is a mem leak! :o
-       * The class is trivially small, but there's one instantiation for each
-       * section delegated to CkMulticast. The objects need to live as long as
-       * their section exists and is used. The idea of 'destroying' an array
-       * section is still academic, and hence no effort has been made to charge
-       * some 'owner' entity with the task of deleting this object.
-       *
-       * Reimplementing delegated x-array reductions will make this consideration moot
-       */
-      // Configure the final cross-section reducer
-      ck::impl::XArraySectionReducer *red =
-          new ck::impl::XArraySectionReducer(numSubSections, cb);
-      // Configure the subsection callback to deposit with the final reducer
-      sectionCB = new CkCallback(ck::impl::processSectionContribution, red);
+    /** @warning: setReductionClient in cross section reduction should be phased out
+     * Redmine Issue: https://charm.cs.illinois.edu/redmine/issues/1249
+     * Since finalCB is already set in initDelegateMgr it needs to be deleted here
+     * There will still be a memory leak for the instantiation of XGroupSectionReducer object
+     * Since this function will eventually be phased out, it is only a quick fix
+     */
+    // Configure the final cross-section reducer
+    CkSectionInfo &sInfo = proxy.ckGetSectionID(0)._cookie;
+    mCastEntry *entry = (mCastEntry *)sInfo.get_val();
+    delete entry->red.storedCallback;
+    ck::impl::XGroupSectionReducer *red =
+      new ck::impl::XGroupSectionReducer(numSubSections, cb);
+    // Configure the subsection callback to deposit with the final reducer
+    sectionCB = new CkCallback(ck::impl::processSectionContribution, red);
   }
   // else, just direct the reduction to the actual client cb
   else
@@ -1198,16 +1219,16 @@ void CkMulticastMgr::setReductionClient(CProxySection_Group &proxy, CkCallback *
   // If its a cross-array section,
   if (numSubSections > 1)
   {
-    /** @warning: Each instantiation is a mem leak! :o
-     * The class is trivially small, but there's one instantiation for each
-     * section delegated to CkMulticast. The objects need to live as long as
-     * their section exists and is used. The idea of 'destroying' an array
-     * section is still academic, and hence no effort has been made to charge
-     * some 'owner' entity with the task of deleting this object.
-     *
-     * Reimplementing delegated x-array reductions will make this consideration moot
+    /** @warning: setReductionClient in cross section reduction should be phased out
+     * Redmine Issue: https://charm.cs.illinois.edu/redmine/issues/1249
+     * Since finalCB is already set in initDelegateMgr it needs to be deleted here
+     * There will still be a memory leak for the instantiation of XGroupSectionReducer object
+     * Since this function will eventually be phased out, it is only a quick fix
      */
     // Configure the final cross-section reducer
+    CkSectionInfo &sInfo = proxy.ckGetSectionID(0)._cookie;
+    mCastEntry *entry = (mCastEntry *)sInfo.get_val();
+    delete entry->red.storedCallback;
     ck::impl::XGroupSectionReducer *red =
       new ck::impl::XGroupSectionReducer(numSubSections, cb);
     // Configure the subsection callback to deposit with the final reducer
@@ -1443,10 +1464,10 @@ void CkMulticastMgr::reduceFragment (int index, CkSectionInfo& id,
             // Set the reference number based on the user flag at the contribute call
             CkSetRefNum(newmsg, userFlag);
             // Trigger the appropriate reduction client
-            if ( !msg_cb.isInvalid() )
-                msg_cb.send(newmsg);
-            else if (redInfo.storedCallback != NULL)
+            if (redInfo.storedCallback != NULL)
                 redInfo.storedCallback->send(newmsg);
+            else if ( !msg_cb.isInvalid() )
+                msg_cb.send(newmsg);
             else if (redInfo.storedClient != NULL) {
                 redInfo.storedClient(id, redInfo.storedClientParam, dataSize, newmsg->data);
                 delete newmsg;