0029064: Copying of empty NCollection map takes excessive memory

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

src/NCollection/NCollection_DataMap.hxx

@@ -168,10 +168,14 @@ public:
       return *this;
 
     Clear();
-    ReSize (theOther.Extent()-1);
+    Standard_Integer anExt = theOther.Extent();
-    Iterator anIter(theOther);
+    if (anExt)
-    for (; anIter.More(); anIter.Next())
+    {
-      Bind (anIter.Key(), anIter.Value());
+      ReSize (anExt-1);
+      Iterator anIter(theOther);
+      for (; anIter.More(); anIter.Next())
+        Bind (anIter.Key(), anIter.Value());
+    }
     return *this;
   }
 

src/NCollection/NCollection_DoubleMap.hxx

@@ -151,20 +151,24 @@ public:
       return *this;
 
     Clear();
-    ReSize (theOther.Extent()-1);
+    Standard_Integer anExt = theOther.Extent();
-    Iterator anIter(theOther);
+    if (anExt)
-    for (; anIter.More(); anIter.Next())
     {
-      TheKey1Type aKey1 = anIter.Key1();
+      ReSize (anExt-1);
-      TheKey2Type aKey2 = anIter.Key2();
+      Iterator anIter(theOther);
-      Standard_Integer iK1 = Hasher1::HashCode (aKey1, NbBuckets());
+      for (; anIter.More(); anIter.Next())
-      Standard_Integer iK2 = Hasher2::HashCode (aKey2, NbBuckets());
+      {
-      DoubleMapNode * pNode = new (this->myAllocator) DoubleMapNode (aKey1, aKey2, 
+        TheKey1Type aKey1 = anIter.Key1();
-                                                                     myData1[iK1], 
+        TheKey2Type aKey2 = anIter.Key2();
-                                                                     myData2[iK2]);
+        Standard_Integer iK1 = Hasher1::HashCode (aKey1, NbBuckets());
-      myData1[iK1] = pNode;
+        Standard_Integer iK2 = Hasher2::HashCode (aKey2, NbBuckets());
-      myData2[iK2] = pNode;
+        DoubleMapNode * pNode = new (this->myAllocator) DoubleMapNode (aKey1, aKey2, 
-      Increment();
+          myData1[iK1], 
+          myData2[iK2]);
+        myData1[iK1] = pNode;
+        myData2[iK2] = pNode;
+        Increment();
+      }
     }
     return *this;
   }

src/NCollection/NCollection_IndexedDataMap.hxx

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

src/NCollection/NCollection_IndexedMap.hxx

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

src/NCollection/NCollection_Map.hxx

@@ -147,10 +147,14 @@ public:
       return *this;
 
     Clear();
-    ReSize (theOther.Extent()-1);
+    int anExt = theOther.Extent();
-    Iterator anIter(theOther);
+    if (anExt)
-    for (; anIter.More(); anIter.Next())
+    {
-      Add (anIter.Key());
+      ReSize (anExt-1);
+      Iterator anIter(theOther);
+      for (; anIter.More(); anIter.Next())
+        Add (anIter.Key());
+    }
     return *this;
   }
 

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;
 }

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