[mlpack-git] master, mlpack-1.0.x: bug fixes for memory leaks (d881abd)

gitdub at big.cc.gt.atl.ga.us gitdub at big.cc.gt.atl.ga.us
Thu Mar 5 21:49:29 EST 2015


Repository : https://github.com/mlpack/mlpack

On branches: master,mlpack-1.0.x
Link       : https://github.com/mlpack/mlpack/compare/904762495c039e345beba14c1142fd719b3bd50e...f94823c800ad6f7266995c700b1b630d5ffdcf40

>---------------------------------------------------------------

commit d881abd25e5439b01f7c71f114952c8425884cc0
Author: andrewmw94 <andrewmw94 at gmail.com>
Date:   Tue Jun 24 16:04:58 2014 +0000

    bug fixes for memory leaks


>---------------------------------------------------------------

d881abd25e5439b01f7c71f114952c8425884cc0
 .../core/tree/rectangle_tree/r_tree_split_impl.hpp | 61 ++++++++++++++++------
 .../tree/rectangle_tree/rectangle_tree_impl.hpp    | 12 ++---
 src/mlpack/methods/neighbor_search/allknn_main.cpp |  1 +
 3 files changed, 52 insertions(+), 22 deletions(-)

diff --git a/src/mlpack/core/tree/rectangle_tree/r_tree_split_impl.hpp b/src/mlpack/core/tree/rectangle_tree/r_tree_split_impl.hpp
index fc26c8f..79f75b4 100644
--- a/src/mlpack/core/tree/rectangle_tree/r_tree_split_impl.hpp
+++ b/src/mlpack/core/tree/rectangle_tree/r_tree_split_impl.hpp
@@ -31,13 +31,14 @@ void RTreeSplit<DescentType, StatisticType, MatType>::SplitLeafNode(
   // If we are splitting the root node, we need will do things differently so that the constructor
   // and other methods don't confuse the end user by giving an address of another node.
   if(tree->Parent() == NULL) {
-    RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType> copy = *tree; // We actually want to copy this way.  Pointers and everything.
+    RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>* copy =
+      new RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>(*tree); // We actually want to copy this way.  Pointers and everything.
       std::cout << "copy made ." << std::endl;
 
-    copy.Parent() = tree;
+    copy->Parent() = tree;
     tree->Count() = 0;
-    tree->Children()[(tree->NumChildren())++] = &copy; // Because this was a leaf node, numChildren must be 0.
-    RTreeSplit<DescentType, StatisticType, MatType>::SplitLeafNode(&copy);
+    tree->Children()[(tree->NumChildren())++] = copy; // Because this was a leaf node, numChildren must be 0.
+    RTreeSplit<DescentType, StatisticType, MatType>::SplitLeafNode(copy);
     std::cout << "finished split" << std::endl;
     return;
   }
@@ -82,15 +83,17 @@ void RTreeSplit<DescentType, StatisticType, MatType>::SplitLeafNode(
 
       
   //because we copied the points to treeOne and treeTwo, we can just delete this node
-  // I THINK?
+  // I THOUGHT?
   //delete tree;
 
   // we only add one at a time, so we should only need to test for equality
   // just in case, we use an assert.
   assert(par->NumChildren() <= par->MaxNumChildren());
   if(par->NumChildren() == par->MaxNumChildren()) {
+    std::cout << "leaf split calls non-leaf split" << std::endl;
     SplitNonLeafNode(par);
   }
+  std::cout << "about to end leaf split." << std::endl;
   return;
 }
 
@@ -107,28 +110,43 @@ template<typename DescentType,
 bool RTreeSplit<DescentType, StatisticType, MatType>::SplitNonLeafNode(
   RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>* tree)
 {
+  std::cout << "splitting non-leaf node." << std::endl;
+
+  // If we are splitting the root node, we need will do things differently so that the constructor
+  // and other methods don't confuse the end user by giving an address of another node.
+  if(tree->Parent() == NULL) {
+    std::cout << "root node" << std::endl;
+    RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>* copy =
+      new RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>(*tree); // We actually want to copy this way.  Pointers and everything.
+    copy->Parent() = tree;
+    tree->NumChildren() = 0;
+    tree->Children()[(tree->NumChildren())++] = copy;
+    RTreeSplit<DescentType, StatisticType, MatType>::SplitNonLeafNode(copy);
+    
+    std::cout << tree->ToString() << std::endl;
+    std::cout << "root split finished" << std::endl;
+    
+    return true;
+  }
+  
+  std::cout << "about to get bound seeds" << std::endl;
   int i = 0;
   int j = 0;
   GetBoundSeeds(*tree, &i, &j);
   
+  std::cout << "bound seeds" << std::endl;
+  
   RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>* treeOne = new 
     RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>(tree->Parent());
   RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>* treeTwo = new 
     RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>(tree->Parent());
   
+  std::cout << "new nodes created" << std::endl;
+
   // This will assign the ith and jth rectangles appropriately.
   AssignNodeDestNode(tree, treeOne, treeTwo, i, j);
   
-  // If we are splitting the root node, we need will do things differently so that the constructor
-  // and other methods don't confuse the end user by giving an address of another node.
-  if(tree->Parent() == NULL) {
-    RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType> copy = *tree; // We actually want to copy this way.  Pointers and everything.
-    copy.Parent() = tree;
-    tree->NumChildren() = 0;
-    tree->Children()[(tree->NumChildren())++] = &copy;
-    RTreeSplit<DescentType, StatisticType, MatType>::SplitNonLeafNode(&copy);
-    return true;
-  }
+  std::cout << "nodes assigned" << std::endl;
   
   //Remove this node and insert treeOne and treeTwo
   RectangleTree<RTreeSplit<DescentType, StatisticType, MatType>, DescentType,  StatisticType, MatType>* par = tree->Parent();
@@ -144,7 +162,7 @@ bool RTreeSplit<DescentType, StatisticType, MatType>::SplitNonLeafNode(
 
   // Because we now have pointers to the information stored under this tree,
   // we need to delete this node carefully.
-  tree->softDelete();
+  tree->softDelete(); //currently does nothing but leak memory.
 
   // we only add one at a time, so should only need to test for equality
   // just in case, we use an assert.
@@ -153,6 +171,17 @@ bool RTreeSplit<DescentType, StatisticType, MatType>::SplitNonLeafNode(
   if(par->NumChildren() == par->MaxNumChildren()) {
     SplitNonLeafNode(par);
   }
+  
+  // We have to update the children of each of these new nodes so that they record the 
+  // correct parent.
+  for(int i = 0; i < treeOne->NumChildren(); i++) {
+    treeOne->Children()[i]->Parent() = treeOne;
+  }
+  for(int i = 0; i < treeTwo->NumChildren(); i++) {
+    treeTwo->Children()[i]->Parent() = treeTwo;
+  }  
+  
+  std::cout << "about to end split non-leaf" << std::endl;
   return false;
 }
 
diff --git a/src/mlpack/core/tree/rectangle_tree/rectangle_tree_impl.hpp b/src/mlpack/core/tree/rectangle_tree/rectangle_tree_impl.hpp
index 5115c7c..1624109 100644
--- a/src/mlpack/core/tree/rectangle_tree/rectangle_tree_impl.hpp
+++ b/src/mlpack/core/tree/rectangle_tree/rectangle_tree_impl.hpp
@@ -48,14 +48,12 @@ RectangleTree<SplitType, DescentType, StatisticType, MatType>::RectangleTree(
   
   // For now, just insert the points in order.
   RectangleTree* root = this;
-  for(int i = firstDataIndex; i < 54 /*data.n_cols*/; i++) {
+  
+  //for(int i = firstDataIndex; i < 57; i++) { // 56,57 are the bound for where it works/breaks
+  for(int i = firstDataIndex; i < data.n_rows; i++) {
     std::cout << "inserting point number: " << i << std::endl;
     root->InsertPoint(data.col(i));
     std::cout << "finished inserting point number: " << i << std::endl;
-    if(root->Parent() != NULL) {
-      root = root->Parent(); // OK since the level increases by at most one per iteration.
-    }
-    std::cout << "hi" << std::endl;
     std::cout << ToString() << std::endl;
   }
   
@@ -95,6 +93,8 @@ template<typename SplitType,
 RectangleTree<SplitType, DescentType, StatisticType, MatType>::
   ~RectangleTree()
 {
+  //LEAK MEMORY
+  
   for(int i = 0; i < numChildren; i++) {
     delete children[i];
   }
@@ -363,7 +363,7 @@ std::string RectangleTree<SplitType, DescentType, StatisticType, MatType>::ToStr
   convert << "  Parent address: " << parent << std::endl;
 
   // How many levels should we print?  This will print the root and it's children.
-  if(parent == NULL) {
+  if(parent == NULL || parent->Parent() == NULL) {
     for(int i = 0; i < numChildren; i++) {
       convert << children[i]->ToString();
     }
diff --git a/src/mlpack/methods/neighbor_search/allknn_main.cpp b/src/mlpack/methods/neighbor_search/allknn_main.cpp
index 11c89b3..0541470 100644
--- a/src/mlpack/methods/neighbor_search/allknn_main.cpp
+++ b/src/mlpack/methods/neighbor_search/allknn_main.cpp
@@ -278,6 +278,7 @@ int main(int argc, char *argv[])
                       arma::mat>
         refTree(referenceData, leafSize, leafSize/3, 5, 2, 0);
       Timer::Stop("tree_building");
+      std::cout << "completed tree building" << std::endl;
     }
   }
   else // Cover trees.



More information about the mlpack-git mailing list