[GitHub] lanking520 commented on a change in pull request #11885: Fix JNI custom op code from deregistering the operator fixes #10438

[GitBox <gi...@apache.org>]

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:
users@infra.apache.org


With regards,
Apache Git Services