[1533] in Coldmud discussion meeting

[patch] optimize method cache

daemon@ATHENA.MIT.EDU (Tue Dec 21 03:01:52 1999 )

From: bruce@puremagic.com
To: coldstuff@cold.org
Date: Mon, 20 Dec 1999 23:51:28 -0800 (PST)
Reply-To: coldstuff@cold.org

This is a patch to help optimize the method cache some.

It changes the following:

1) Do not invalidate the cache when destroying an object that has
   no methods and no children.  This'll prevent the method cache
   from getting nuked everytime someone connects as a user, or via
   the web interface.
2) When deleting a method, only invalidate if you actually do
   anything.  In any other case, why bother invalidating?
3) In setting the access on a method, only invalidate in the
   specific instance in which you need to invalidate.  Also,
   save some work and don't flag it as dirty unless you need to
   do so.

There are other optimizations left that could/should be made.  I had made
posts about this to the genesis@cold.org mailing list, nearly a year ago:

http://web.cold.org/messaging/genesis/msg00003.html
http://web.cold.org/messaging/genesis/msg00004.html

The stuff in there is still true.  The first fix above isn't optimal (for
reasons laid out below, but I don't feel like thinking about it that hard
tonight).

The other optimization that should be addressed and that was left out of
both of those emails, is that negative method lookups should be cached.
However, with the addition of that much extra information to the cache,
it is well past time for someone to look at adding monitoring data to the
various caches and tracking the hit/miss/overwrite rates to help people
tune their cache sizes.

 - Bruce

--- Genesis-1.1.8-STABLE/src/data/object.c	Tue Jul 13 13:47:30 1999
+++ Genesis/src/data/object.c	Mon Dec 20 23:30:07 1999
@@ -145,9 +145,19 @@
     cList *children;
     cData *d, cthis;
     Obj *kid;
+    Long has_methods = 0, i = 0;
 
-    /* Invalidate the method cache. */
-    cur_stamp++;
+    do {
+        if (object->methods.tab[i].m) {
+            has_methods = 1;
+        }
+	i++;
+    } while (!has_methods && (i < object->methods.size));
+
+    /* Invalidate the method cache if object is not a method-less leaf object */
+    if ((list_length(object->children) != 0 || (has_methods)) {
+        cur_stamp++;
+    }
 
     /* remove the object name, if it has one */
     object_del_objname(object);
@@ -1085,9 +1095,6 @@
 Int object_del_method(Obj *object, Long name) {
     Int *indp, ind;
 
-    /* Invalidate the method cache. */
-    cur_stamp++;
-
     /* This is the index-thread equivalent of double pointers in a standard
      * linked list.  We traverse the list using pointers to the ->next element
      * of the method pointers. */
@@ -1114,6 +1121,9 @@
 
 	    object->dirty = 1;
 
+	    /* Invalidate method cache */
+	    cur_stamp++;
+
 	    /* Return one, meaning the method was successfully deleted. */
 	    return 1;
 	}
@@ -1162,9 +1172,18 @@
     method = object_find_method_local(object, name, FROB_ANY);
     if (method == NULL)
         return -1;
+    if (method->m_access = access) {
+        /* yay, we don't have to do anything. let's go home. */
+        return access;
+    }
+    if ((method->m_access == MS_FROB) || (access == MS_FROB)) {
+        /*
+	 * only invalidate when changing to or from 'frob' access.
+	 */
+	cur_stamp++;
+    }
     method->m_access = access;
     object->dirty = 1;
-    cur_stamp++; /* to invalidate cached frob/!frob defaults */
     return access;
 }