changeset 194:e761d94faf34

Bug 3296: Reference counter should manage TObjectData instance Reviewed-by: yasuenag GitHub: https://github.com/HeapStats/heapstats/pull/80
author KUBOTA Yuji <kubota.yuji@lab.ntt.co.jp>
date Tue, 24 Jan 2017 11:46:41 +0900
parents 21afb593dd95
children 61e865edcf55
files ChangeLog agent/src/heapstats-engines/classContainer.cpp agent/src/heapstats-engines/snapShotContainer.cpp agent/src/heapstats-engines/snapShotContainer.hpp
diffstat 4 files changed, 32 insertions(+), 13 deletions(-) [+]
line wrap: on
line diff
--- a/ChangeLog	Mon Jan 23 21:48:48 2017 +0900
+++ b/ChangeLog	Tue Jan 24 11:46:41 2017 +0900
@@ -1,3 +1,7 @@
+2017-01-24 KUBOTA Yuji <kubota.yuji@lab.ntt.co.jp>
+
+	* Bug 3296: Reference counter should manage TObjectData instance
+
 2017-01-23  Yasumasa Suenaga <yasuenag@gmail.com>
 
 	* Bug 3294: Reference counter should be handled in TClassContainer::allClear()
--- a/agent/src/heapstats-engines/classContainer.cpp	Mon Jan 23 21:48:48 2017 +0900
+++ b/agent/src/heapstats-engines/classContainer.cpp	Tue Jan 24 11:46:41 2017 +0900
@@ -255,6 +255,10 @@
                      strcmp(objData->className, expectData->className) == 0 &&
                      objData->clsLoaderId == expectData->clsLoaderId)) {
           /* Return existing data on map. */
+          /*
+           * We should not increment reference counter because we do not add
+           * reference.
+           */
           existData = expectData;
         } else {
           /* klass oop is doubling for another class. */
@@ -275,6 +279,7 @@
       try {
         /* Append class data. */
         (*classMap)[klassOop] = objData;
+        atomic_inc(&objData->numRefs, 1);
       } catch (...) {
         /*
          * Maybe failed to allocate memory at "std::map::operator[]".
@@ -296,7 +301,10 @@
     /* Broadcast to each local container. */
     for (TLocalClassContainer::iterator it = localContainers.begin();
          it != localContainers.end(); it++) {
-      (*it)->pushNewClass(klassOop, objData);
+      // We should skip myself if "this" ptr is in local container.
+      if (*it != this) {
+        (*it)->pushNewClass(klassOop, objData);
+      }
     }
   }
   /* Release spin lock of containers queue. */
@@ -323,6 +331,7 @@
 void TClassContainer::removeClass(TObjectData *target) {
   /* Remove item from map. Please callee has container's lock. */
   classMap->erase(target->klassOop);
+  atomic_inc(&target->numRefs, -1);
 
   /* Get spin lock of containers queue. */
   spinLockWait(&queueLock);
@@ -330,11 +339,17 @@
     /* Broadcast to each local container. */
     for (TLocalClassContainer::iterator it = localContainers.begin();
          it != localContainers.end(); it++) {
-      /* Get local container's spin lock. */
-      spinLockWait(&(*it)->lockval);
-      { (*it)->classMap->erase(target->klassOop); }
-      /* Release local container's spin lock. */
-      spinLockRelease(&(*it)->lockval);
+      // We should skip myself if "this" ptr is in local container.
+      if (*it != this) {
+        /* Get local container's spin lock. */
+        spinLockWait(&(*it)->lockval);
+        {
+          (*it)->classMap->erase(target->klassOop);
+          atomic_inc(&target->numRefs, -1);
+        }
+        /* Release local container's spin lock. */
+        spinLockRelease(&(*it)->lockval);
+      }
     }
   }
   /* Release spin lock of containers queue. */
@@ -369,7 +384,7 @@
          ++cur) {
       TObjectData *pos = (*cur).second;
 
-      if ((pos != NULL) && (atomic_get(&pos->numRefsFromChildren) == 0)) {
+      if ((pos != NULL) && (atomic_get(&pos->numRefs) == 0)) {
         free(pos->className);
         free(pos);
       }
@@ -977,7 +992,7 @@
 
         /* If class is prepared remove from class container. */
         if (unlikely(objData->isRemoved &&
-                     (atomic_get(&objData->numRefsFromChildren) == 0))) {
+                     (atomic_get(&objData->numRefs) == 0))) {
           /*
            * If we do removing map item here,
            * iterator's relationship will be broken.
--- a/agent/src/heapstats-engines/snapShotContainer.cpp	Mon Jan 23 21:48:48 2017 +0900
+++ b/agent/src/heapstats-engines/snapShotContainer.cpp	Tue Jan 24 11:46:41 2017 +0900
@@ -205,7 +205,7 @@
       counter = counter->next;
 
       /* Deallocate TChildClassCounter. */
-      atomic_inc(&aCounter->objData->numRefsFromChildren, -1);
+      atomic_inc(&aCounter->objData->numRefs, -1);
       free(aCounter->counter);
       free(aCounter);
     }
@@ -300,7 +300,7 @@
     return NULL;
   }
 
-  atomic_inc(&objData->numRefsFromChildren, 1);
+  atomic_inc(&objData->numRefs, 1);
   this->clearObjectCounter(newCounter->counter);
   newCounter->objData = objData;
 
@@ -481,7 +481,7 @@
            * TClassContainer::commitClassChange().
            */
           if (objData->isRemoved) {
-            atomic_inc(&objData->numRefsFromChildren, -1);
+            atomic_inc(&objData->numRefs, -1);
             TChildClassCounter *nextCounter = counter->next;
 
             if (prevCounter == NULL) {
@@ -501,7 +501,7 @@
                                     &childClsData, &parentPrevData,
                                     &parentMorePrevData);
             if (likely(childClsData != NULL)) {
-              atomic_inc(&objData->numRefsFromChildren, -1);
+              atomic_inc(&objData->numRefs, -1);
 
               if (parentPrevData == NULL) {
                 clsCounter->child = childClsData->next;
--- a/agent/src/heapstats-engines/snapShotContainer.hpp	Mon Jan 23 21:48:48 2017 +0900
+++ b/agent/src/heapstats-engines/snapShotContainer.hpp	Tue Jan 24 11:46:41 2017 +0900
@@ -79,7 +79,7 @@
   jlong clsLoaderTag;      /*!< Class loader class tag.                       */
   bool isRemoved;          /*!< Class is already unloaded.                    */
   jlong instanceSize;      /*!< Class size if this class is instanceKlass.    */
-  int numRefsFromChildren; /*!< Number of references from TChildClassCounter. */
+  int numRefs;             /*!< Number of references.                         */
 } TObjectData;
 #pragma pack(pop)