0029064: Copying of empty NCollection map takes excessive memory

Avoid resizing of NCollection maps in Assign() methods if source map is empty

diff --git a/src/NCollection/NCollection_DataMap.hxx b/src/NCollection/NCollection_DataMap.hxx
index e3bdb3fb2deed7f0eb3c2bb2afebbaa60f746f0d..ba22e36d429f1d899358da2c054277b8152aba71 100644 (file)
--- a/src/NCollection/NCollection_DataMap.hxx
+++ b/src/NCollection/NCollection_DataMap.hxx
@@ -168,10 +168,14 @@ public:
       return *this;
 
     Clear();
-    ReSize (theOther.Extent()-1);
-    Iterator anIter(theOther);
-    for (; anIter.More(); anIter.Next())
-      Bind (anIter.Key(), anIter.Value());
+    Standard_Integer anExt = theOther.Extent();
+    if (anExt)
+    {
+      ReSize (anExt-1);
+      Iterator anIter(theOther);
+      for (; anIter.More(); anIter.Next())
+        Bind (anIter.Key(), anIter.Value());
+    }
     return *this;
   }
 

diff --git a/src/NCollection/NCollection_DoubleMap.hxx b/src/NCollection/NCollection_DoubleMap.hxx
index 5f1743423e4eaee68fe3cedd9abffa429d0ea059..993e7a883119b00663380d4504c8eeb0d8eedd1e 100644 (file)
--- a/src/NCollection/NCollection_DoubleMap.hxx
+++ b/src/NCollection/NCollection_DoubleMap.hxx
@@ -151,20 +151,24 @@ public:
       return *this;
 
     Clear();
-    ReSize (theOther.Extent()-1);
-    Iterator anIter(theOther);
-    for (; anIter.More(); anIter.Next())
+    Standard_Integer anExt = theOther.Extent();
+    if (anExt)
     {
-      TheKey1Type aKey1 = anIter.Key1();
-      TheKey2Type aKey2 = anIter.Key2();
-      Standard_Integer iK1 = Hasher1::HashCode (aKey1, NbBuckets());
-      Standard_Integer iK2 = Hasher2::HashCode (aKey2, NbBuckets());
-      DoubleMapNode * pNode = new (this->myAllocator) DoubleMapNode (aKey1, aKey2, 
-                                                                     myData1[iK1], 
-                                                                     myData2[iK2]);
-      myData1[iK1] = pNode;
-      myData2[iK2] = pNode;
-      Increment();
+      ReSize (anExt-1);
+      Iterator anIter(theOther);
+      for (; anIter.More(); anIter.Next())
+      {
+        TheKey1Type aKey1 = anIter.Key1();
+        TheKey2Type aKey2 = anIter.Key2();
+        Standard_Integer iK1 = Hasher1::HashCode (aKey1, NbBuckets());
+        Standard_Integer iK2 = Hasher2::HashCode (aKey2, NbBuckets());
+        DoubleMapNode * pNode = new (this->myAllocator) DoubleMapNode (aKey1, aKey2, 
+          myData1[iK1], 
+          myData2[iK2]);
+        myData1[iK1] = pNode;
+        myData2[iK2] = pNode;
+        Increment();
+      }
     }
     return *this;
   }

diff --git a/src/NCollection/NCollection_IndexedDataMap.hxx b/src/NCollection/NCollection_IndexedDataMap.hxx
index b6c3684023449cedacd598094887b74d17a8ae29..7f1f823b8b762e07d54b113c721fd59cf1f8784d 100644 (file)
--- a/src/NCollection/NCollection_IndexedDataMap.hxx
+++ b/src/NCollection/NCollection_IndexedDataMap.hxx
@@ -190,16 +190,20 @@ private:
       return *this;
 
     Clear();
-    ReSize (theOther.Extent()-1);
-    for (Standard_Integer anIndexIter = 1; anIndexIter <= theOther.Extent(); ++anIndexIter)
+    Standard_Integer anExt = theOther.Extent();
+    if (anExt)
     {
-      const TheKeyType&  aKey1  = theOther.FindKey      (anIndexIter);
-      const TheItemType& anItem = theOther.FindFromIndex(anIndexIter);
-      const Standard_Integer iK1 = Hasher::HashCode (aKey1, NbBuckets());
-      IndexedDataMapNode* pNode = new (this->myAllocator) IndexedDataMapNode (aKey1, anIndexIter, anItem, myData1[iK1]);
-      myData1[iK1]             = pNode;
-      myData2[anIndexIter - 1] = pNode;
-      Increment();
+      ReSize (anExt-1); //mySize is same after resize
+      for (Standard_Integer anIndexIter = 1; anIndexIter <= anExt; ++anIndexIter)
+      {
+        const TheKeyType&  aKey1  = theOther.FindKey      (anIndexIter);
+        const TheItemType& anItem = theOther.FindFromIndex(anIndexIter);
+        const Standard_Integer iK1 = Hasher::HashCode (aKey1, NbBuckets());
+        IndexedDataMapNode* pNode = new (this->myAllocator) IndexedDataMapNode (aKey1, anIndexIter, anItem, myData1[iK1]);
+        myData1[iK1]             = pNode;
+        myData2[anIndexIter - 1] = pNode;
+        Increment();
+      }
     }
     return *this;
   }

diff --git a/src/NCollection/NCollection_IndexedMap.hxx b/src/NCollection/NCollection_IndexedMap.hxx
index c0a66cd4e047b036e8a90370e1dccf0584c4a176..6f4da8f4e94ec06b86da638682e5547595f375e5 100644 (file)
--- a/src/NCollection/NCollection_IndexedMap.hxx
+++ b/src/NCollection/NCollection_IndexedMap.hxx
@@ -152,16 +152,19 @@ private:
       return *this;
 
     Clear();
-    ReSize (theOther.Extent()-1);
-    const Standard_Integer iLength = theOther.Extent();
-    for (Standard_Integer anIndexIter = 1; anIndexIter <= iLength; ++anIndexIter)
+    Standard_Integer anExt = theOther.Extent();
+    if (anExt)
     {
-      const TheKeyType& aKey1 = theOther.FindKey (anIndexIter);
-      const Standard_Integer iK1 = Hasher::HashCode (aKey1, NbBuckets());
-      IndexedMapNode* pNode = new (this->myAllocator) IndexedMapNode (aKey1, anIndexIter, myData1[iK1]);
-      myData1[iK1]             = pNode;
-      myData2[anIndexIter - 1] = pNode;
-      Increment();
+      ReSize (anExt-1); //mySize is same after resize
+      for (Standard_Integer anIndexIter = 1; anIndexIter <= anExt; ++anIndexIter)
+      {
+        const TheKeyType& aKey1 = theOther.FindKey (anIndexIter);
+        const Standard_Integer iK1 = Hasher::HashCode (aKey1, NbBuckets());
+        IndexedMapNode* pNode = new (this->myAllocator) IndexedMapNode (aKey1, anIndexIter, myData1[iK1]);
+        myData1[iK1]             = pNode;
+        myData2[anIndexIter - 1] = pNode;
+        Increment();
+      }
     }
     return *this;
   }

diff --git a/src/NCollection/NCollection_Map.hxx b/src/NCollection/NCollection_Map.hxx
index 459c4f2c7d60d53e0ff146892f435edf46577644..3aa3f70119ee19d4dc79b2e9187e099c253cc5e8 100644 (file)
--- a/src/NCollection/NCollection_Map.hxx
+++ b/src/NCollection/NCollection_Map.hxx
@@ -147,10 +147,14 @@ public:
       return *this;
 
     Clear();
-    ReSize (theOther.Extent()-1);
-    Iterator anIter(theOther);
-    for (; anIter.More(); anIter.Next())
-      Add (anIter.Key());
+    int anExt = theOther.Extent();
+    if (anExt)
+    {
+      ReSize (anExt-1);
+      Iterator anIter(theOther);
+      for (; anIter.More(); anIter.Next())
+        Add (anIter.Key());
+    }
     return *this;
   }
 

diff --git a/src/QABugs/QABugs_20.cxx b/src/QABugs/QABugs_20.cxx
index 80f90678cb795a40112ba7cd1a9f5aede34ff451..8caea5e40dec4efcd8f85a8ef4904b5e0f02f957 100644 (file)
--- a/src/QABugs/QABugs_20.cxx
+++ b/src/QABugs/QABugs_20.cxx
@@ -2768,6 +2768,61 @@ static Standard_Integer OCC29371 (Draw_Interpretor& di, Standard_Integer n, cons
   return 0;
 }
 
+#include <NCollection_DoubleMap.hxx>
+#include <NCollection_IndexedMap.hxx>
+#include <NCollection_DataMap.hxx>
+#include <NCollection_IndexedDataMap.hxx>
+#include <OSD_MemInfo.hxx>
+
+// check that copying of empty maps does not allocate extra memory
+template<typename T> void AllocDummyArr (Draw_Interpretor& theDI, int theN1, int theN2)
+{
+  NCollection_Array1<T> aMapArr1(0, theN1), aMapArr2(0, theN2);
+
+  OSD_MemInfo aMemTool;
+  Standard_Size aMem0 = aMemTool.Value (OSD_MemInfo::MemHeapUsage);
+
+  for (int i = 1; i < theN1; i++)
+    aMapArr1(i) = aMapArr1(i-1);
+  for (int i = 1; i < theN2; i++)
+    aMapArr2(i) = aMapArr2(0);
+
+  aMemTool.Update();
+  Standard_Size aMem1 = aMemTool.Value (OSD_MemInfo::MemHeapUsage);
+
+  theDI << "Heap usage before copy = " << (int)aMem0 << ", after = " << (int)aMem1 << "\n";
+  
+  if (aMem1 > aMem0)
+    theDI << "Error: memory increased by " << (int)(aMem1 - aMem0) << " bytes\n";
+};
+
+static Standard_Integer OCC29064 (Draw_Interpretor& theDI, Standard_Integer theArgc, const char** theArgv)
+{
+  if (theArgc < 2)
+  {
+    cout << "Error: give argument indicating type of map (map, doublemap, datamap, indexedmap, indexeddatamap)" << endl;
+    return 1;
+  }
+
+  const int nbm1 = 10000, nbm2 = 10000;
+  if (strcasecmp (theArgv[1], "map") == 0)
+    AllocDummyArr<NCollection_Map<int> > (theDI, nbm1, nbm2);
+  else if (strcasecmp (theArgv[1], "doublemap") == 0)
+    AllocDummyArr<NCollection_DoubleMap<int, int> > (theDI, nbm1, nbm2);
+  else if (strcasecmp (theArgv[1], "datamap") == 0)
+    AllocDummyArr<NCollection_DataMap<int, int> > (theDI, nbm1, nbm2);
+  else if (strcasecmp (theArgv[1], "indexedmap") == 0)
+    AllocDummyArr<NCollection_IndexedMap<int> > (theDI, nbm1, nbm2);
+  else if (strcasecmp (theArgv[1], "indexeddatamap") == 0)
+    AllocDummyArr<NCollection_IndexedDataMap<int, int> > (theDI, nbm1, nbm2);
+  else
+  {
+    cout << "Error: unrecognized argument " << theArgv[1] << endl;
+    return 1;
+  }
+  return 0;
+}
+
 #include <BRepOffsetAPI_MakePipeShell.hxx>
 #include <GC_MakeArcOfCircle.hxx>
 #include <BRepAdaptor_CompCurve.hxx>
@@ -2869,5 +2924,6 @@ void QABugs::Commands_20(Draw_Interpretor& theCommands) {
                               __FILE__, OCC29430, group);
   theCommands.Add("OCC29531", "OCC29531 <step file name>", __FILE__, OCC29531, group);
 
+  theCommands.Add ("OCC29064", "OCC29064: test memory usage by copying empty maps", __FILE__, OCC29064, group);
   return;
 }

diff --git a/tests/bugs/fclasses/bug29064 b/tests/bugs/fclasses/bug29064
new file mode 100644 (file)
index 0000000..84bffd0
--- /dev/null
+++ b/tests/bugs/fclasses/bug29064
@@ -0,0 +1,12 @@
+puts "========"
+puts " 0029064: Copying of empty NCollection map takes excessive memory"
+puts "========"
+puts ""
+
+pload QAcommands
+
+OCC29064 map
+OCC29064 doublemap
+OCC29064 datamap
+OCC29064 indexedmap
+OCC29064 indexeddatamap