lanking520 commented on a change in pull request #11885: Fix JNI custom op code 
from deregistering the operator fixes #10438
URL: https://github.com/apache/incubator-mxnet/pull/11885#discussion_r205962252
 
 

 ##########
 File path: 
scala-package/native/src/main/native/org_apache_mxnet_native_c_api.cc
 ##########
 @@ -2419,39 +2416,14 @@ JNIEXPORT jint JNICALL 
Java_org_apache_mxnet_LibInfo_mxCustomOpRegister
 
     // del callback
     auto opPropDel = [](void *state) {
-      std::string key(reinterpret_cast<char *>(state));
-      std::unique_lock<std::mutex> lock(mutex_opprop);
-      int count_prop = globalOpPropCountMap.at(key);
-      if (count_prop < 2) {
-        globalOpPropCountMap[key] = ++count_prop;
-        return 1;
-      }
-      int success = true;
-      if (globalOpPropMap.find(key) == globalOpPropMap.end()) {
-        LOG(WARNING) << "opProp: " << key << " not found";
-        success = false;
-      } else {
-        JNIEnv *env;
-        _jvm->AttachCurrentThread(reinterpret_cast<void **>(&env), NULL);
-        env->DeleteGlobalRef(globalOpPropMap.at(key));
-        _jvm->DetachCurrentThread();
-        for (auto it = globalOpPropMap.begin(); it != globalOpPropMap.end(); ) 
{
-          if (it->first == key) {
-            it = globalOpPropMap.erase(it);
-          } else {
-            ++it;
-          }
-        }
-        for (auto it = globalOpPropCountMap.begin(); it != 
globalOpPropCountMap.end(); ) {
-          if (it->first == key) {
-            it = globalOpPropCountMap.erase(it);
-          } else {
-            ++it;
-          }
-        }
-      }
-      lock.unlock();
-      return success;
+      /*
+       * This method seems to be called by the engine to clean up after 
multiple calls were made
+       * to the creator lambda. The current creator function isn't allocating 
a new object but is
+       * instead reinitializing the object which was created when register was 
called. This means
+       * that there doesn't seem to be anything to clean up here (previous 
efforts were actually
+       * deregistering the operator).
+      */
 
 Review comment:
   As you mentioned in here, have you observe any memory leaks in here if we 
got the counter part removed?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
[email protected]


With regards,
Apache Git Services

Reply via email to