lib: fix MemoryUsage for positional and component objects

Fixes #897

lib/object.gi

@@ -763,27 +763,44 @@ InstallGlobalFunction( MU_Finalize, function (  )
   fi;
 end );
 
-InstallMethod( MemoryUsage, "fallback method for objs without subobjs",
+InstallMethod( MemoryUsage, "generic fallback method",
   [ IsObject ],
   function( o )
-    local mem;
-    mem := SHALLOW_SIZE(o);
-    if mem = 0 then 
+    local mem,known,i,s;
+
+    if SHALLOW_SIZE(o) = 0 then
         return MU_MemPointer;
+    fi;
+
+    if MU_AddToCache( o ) then
+        return 0;    # already counted
+    fi;
+
+    MEMUSAGECACHE.depth := MEMUSAGECACHE.depth + 1;
+
+    # Count the bag, the header, and the master pointer
+    mem := SHALLOW_SIZE(o) + MU_MemBagHeader + MU_MemPointer;
+    if IS_POSOBJ(o) then
+        # Again the bag, its header, and the master pointer
+        for i in [1..LEN_POSOBJ(o)] do
+            if IsBound(o![i]) then
+                if SHALLOW_SIZE(o![i]) > 0 then    # a subobject!
+                    mem := mem + MemoryUsage(o![i]);
+                fi;
+            fi;
+        od;
+    elif IS_COMOBJ(o) then
+        # Again the bag, its header, and the master pointer
+        for i in NamesOfComponents(o) do
+            s := o!.(i);
+            if SHALLOW_SIZE(s) > 0 then    # a subobject!
+                mem := mem + MemoryUsage(s);
+            fi;
+        od;
     else
-        # a proper object, thus we have to add it to the database
-        # to not count it again!
-        if not(MU_AddToCache(o)) then
-            mem := mem + MU_MemBagHeader + MU_MemPointer;
-            # This is for the bag, the header, and the master pointer
-        else 
-            mem := 0;   # already counted
-        fi;
-        if MEMUSAGECACHE.depth = 0 then   
-            # we were the first to be called, thus we have to do the cleanup
-            ClearObjectMarker( MEMUSAGECACHE );
-        fi;
+        # objs without subobjs
     fi;
+    MU_Finalize();
     return mem;
   end );
 
@@ -830,49 +847,6 @@ InstallMethod( MemoryUsage, "for a record",
     return 0;    # already counted
   end );
 
-InstallMethod( MemoryUsage, "for a positional object",
-  [ IsPositionalObjectRep ],
-  function( o )
-    local mem,known,i;
-    known := MU_AddToCache( o );
-    if known = false then    # not yet known
-        MEMUSAGECACHE.depth := MEMUSAGECACHE.depth + 1;
-        mem := SHALLOW_SIZE(o) + MU_MemBagHeader + MU_MemPointer;
-        # Again the bag, its header, and the master pointer
-        for i in [1..(SHALLOW_SIZE(o)/MU_MemPointer)-1] do
-            if IsBound(o![i]) then
-                if SHALLOW_SIZE(o![i]) > 0 then    # a subobject!
-                    mem := mem + MemoryUsage(o![i]);
-                fi;
-            fi;
-        od;
-        MU_Finalize();
-        return mem;
-    fi;
-    return 0;    # already counted
-  end );
-
-InstallMethod( MemoryUsage, "for a component object",
-  [ IsComponentObjectRep ],
-  function( o )
-    local mem,known,i,s;
-    known := MU_AddToCache( o );
-    if known = false then    # not yet known
-        MEMUSAGECACHE.depth := MEMUSAGECACHE.depth + 1;
-        mem := SHALLOW_SIZE(o) + MU_MemBagHeader + MU_MemPointer;
-        # Again the bag, its header, and the master pointer
-        for i in NamesOfComponents(o) do
-            s := o!.(i);
-            if SHALLOW_SIZE(s) > 0 then    # a subobject!
-                mem := mem + MemoryUsage(s);
-            fi;
-        od;
-        MU_Finalize();
-        return mem;
-    fi;
-    return 0;    # already counted
-  end );
-
 InstallMethod( MemoryUsage, "for a rational",
   [ IsRat ],
   function( o )

tst/testinstall/opers/MemoryUsage.tst

@@ -0,0 +1,111 @@
+gap> START_TEST("MemoryUsage.tst");
+
+#
+# test internal objects
+#
+gap> MemoryUsage(42) / GAPInfo.BytesPerVariable;
+1
+gap> (MemoryUsage(2/3) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+3
+gap> str := "abcd";;
+gap> (MemoryUsage(str) - Length(str) - 1 - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+2
+gap> MemoryUsage(Z(2)) / GAPInfo.BytesPerVariable;
+1
+gap> MemoryUsage(Z(3)) / GAPInfo.BytesPerVariable;
+1
+gap> g := (1,2,3);;
+gap> MemoryUsage(g) - MU_MemBagHeader - MU_MemPointer;
+6
+
+#
+# test records
+#
+gap> (MemoryUsage(rec()) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+3
+gap> (MemoryUsage(rec(a:=0)) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+5
+
+#
+# test plain lists
+#
+gap> (MemoryUsage([]) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+2
+gap> (MemoryUsage([1]) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+3
+gap> (MemoryUsage([1,2]) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+4
+gap> (MemoryUsage([1,2,3]) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+5
+gap> (MemoryUsage([1,,3]) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+5
+
+#
+# test self referential records and lists
+gap> MemoryUsage([~]) = MemoryUsage([1]);
+true
+gap> MemoryUsage([0,~]) = MemoryUsage([0,1]);
+true
+gap> MemoryUsage([~,0,~]) = MemoryUsage([0,1,2]);
+true
+gap> MemoryUsage(rec(a:=~)) =MemoryUsage(rec(a:=0));
+true
+
+#
+# test ranges
+#
+gap> (MemoryUsage([1..100]) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+4
+
+#
+# test component objects
+#
+gap> G := Group(g);;
+gap> (MemoryUsage(G) - SHALLOW_SIZE(G) - MemoryUsage(g) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+6
+
+#
+# test families and types
+#
+gap> MemoryUsage(FamilyObj(42));
+0
+gap> MemoryUsage(TypeObj(42));
+0
+
+#
+# test positional objects
+#
+gap> (MemoryUsage(ZmodnZObj(5,8)) - MU_MemBagHeader) / GAPInfo.BytesPerVariable;
+3
+
+#
+gap> F := FreeGroup(3);;
+gap> w := One(F);;
+gap> w:=Product(GeneratorsOfGroup(F))^3;
+(f1*f2*f3)^3
+gap> MemoryUsage(w![1]) = SHALLOW_SIZE(w![1]) + MU_MemBagHeader + MU_MemPointer;
+true
+gap> MemoryUsage(w) = SHALLOW_SIZE(w) + MU_MemBagHeader + MU_MemPointer + MemoryUsage(w![1]);
+true
+gap> o := ExtRepOfObj(w);
+[ 1, 1, 2, 1, 3, 1, 1, 1, 2, 1, 3, 1, 1, 1, 2, 1, 3, 1 ]
+gap> MemoryUsage(o) = SHALLOW_SIZE(o) + MU_MemBagHeader + MU_MemPointer;
+true
+
+#
+# test functions
+#
+gap> MemoryUsage(x -> x) in [ 336, 220 ];
+true
+gap> MemoryUsage(x -> x+1) in [ 384, 256 ];
+true
+
+#
+# bugfix test from 2007/12/14 (MN)
+#
+gap> a := [1..100];;
+gap> MemoryUsage(a) = MemoryUsage(a);
+true
+
+#
+gap> STOP_TEST("MemoryUsage.tst", 10000);

tst/teststandard/bugfix.tst

@@ -1519,11 +1519,6 @@ gap> d:=NewDictionary(3213,true);;
 gap> LookupDictionary(d,4);
 fail
 
-# 2007/12/14 (MN)
-gap> a := [1..100];;
-gap> MemoryUsage(a)=MemoryUsage(a);
-true
-
 # 2008/01/02 (AH)
 gap> G:=SmallGroup(1308,1);
 <pc group of size 1308 with 4 generators>