core: improve performance through negative command caching

Instead of incrementing the proc epoch on every command removal
and some command creation, cache previous deleted commands
(empty structure only). Periodically increment the proc epoch
and invalide all cached commands.

This is especially a win when creating short lived commands.
e.g.

	proc a {} {
		local proc b {} {
			# do something
		}
		# now b is removed
	}

Signed-off-by: Steve Bennett <steveb@workware.net.au>

2 files changed, 49 insertions, 14 deletions

diff --git a/jim.c b/jim.c
index 233dba2..72b6c3f 100644
--- a/jim.c
+++ b/jim.c
@@ -3757,6 +3757,19 @@ static int JimScriptValid(Jim_Interp *interp, ScriptObj *script)
 /* -----------------------------------------------------------------------------
  * Commands
  * ---------------------------------------------------------------------------*/
+void Jim_InterpIncrProcEpoch(Jim_Interp *interp)
+{
+    interp->procEpoch++;
+
+    /* Now discard all out-of-date Jim_Cmd entries */
+    while (interp->oldCmdCache) {
+        Jim_Cmd *next = interp->oldCmdCache->prevCmd;
+        Jim_Free(interp->oldCmdCache);
+        interp->oldCmdCache = next;
+    }
+    interp->oldCmdCacheSize = 0;
+}
+
 static void JimIncrCmdRefCount(Jim_Cmd *cmdPtr)
 {
     cmdPtr->inUse++;
@@ -3784,7 +3797,22 @@ static void JimDecrCmdRefCount(Jim_Interp *interp, Jim_Cmd *cmdPtr)
             /* Delete any pushed command too */
             JimDecrCmdRefCount(interp, cmdPtr->prevCmd);
         }
-        Jim_Free(cmdPtr);
+
+        if (interp->quitting) {
+            Jim_Free(cmdPtr);
+        }
+        else {
+            /* Preserve the structure with inUse = 0 so that
+             * cached references will continue to work.
+             * These will be discarding at the next procEpoch increment
+             * or once 1000 have been accumulated.
+             */
+            cmdPtr->prevCmd = interp->oldCmdCache;
+            interp->oldCmdCache = cmdPtr;
+            if (++interp->oldCmdCacheSize >= 1000) {
+                Jim_InterpIncrProcEpoch(interp);
+            }
+        }
     }
 }
 
@@ -3956,13 +3984,15 @@ static int JimCreateCommand(Jim_Interp *interp, Jim_Obj *nameObjPtr, Jim_Cmd *cm
          * to increment the 'proc epoch' because creation of a new procedure
          * can never affect existing cached commands. We don't do
          * negative caching. */
-        Jim_InterpIncrProcEpoch(interp);
+        //Jim_InterpIncrProcEpoch(interp);
     }
 
     if (he && interp->local) {
         /* Push this command over the top of the previous one */
         cmd->prevCmd = Jim_GetHashEntryVal(he);
         Jim_SetHashVal(&interp->commands, he, cmd);
+        /* Need to increment the proc epoch here so that the new command will be used */
+        Jim_InterpIncrProcEpoch(interp);
     }
     else {
         if (he) {
@@ -4196,9 +4226,6 @@ int Jim_DeleteCommand(Jim_Interp *interp, Jim_Obj *nameObj)
         Jim_SetResultFormatted(interp, "can't delete \"%#s\": command doesn't exist", nameObj);
         ret = JIM_ERR;
     }
-    else {
-        Jim_InterpIncrProcEpoch(interp);
-    }
     Jim_DecrRefCount(interp, nameObj);
 
     return ret;
@@ -4291,18 +4318,23 @@ static const Jim_ObjType commandObjType = {
  */
 Jim_Cmd *Jim_GetCommand(Jim_Interp *interp, Jim_Obj *objPtr, int flags)
 {
-    Jim_Cmd *cmd;
+    Jim_Cmd *cmd = NULL;
 
     /* In order to be valid, the proc epoch must match and
      * the lookup must have occurred in the same namespace
      */
-    if (objPtr->typePtr != &commandObjType ||
-            objPtr->internalRep.cmdValue.procEpoch != interp->procEpoch
+    if (objPtr->typePtr == &commandObjType) {
+        cmd = objPtr->internalRep.cmdValue.cmdPtr;
+        if (cmd->inUse == 0 || objPtr->internalRep.cmdValue.procEpoch != interp->procEpoch
 #ifdef jim_ext_namespace
             || !Jim_StringEqObj(objPtr->internalRep.cmdValue.nsObj, interp->framePtr->nsObj)
 #endif
         ) {
-        /* Not cached or out of date, so lookup */
+            /* Cache is invalid */
+            cmd = NULL;
+        }
+    }
+    if (!cmd) {
         Jim_Obj *qualifiedNameObj = JimQualifyName(interp, objPtr);
         Jim_HashEntry *he = Jim_FindHashEntry(&interp->commands, qualifiedNameObj);
 #ifdef jim_ext_namespace
@@ -4328,9 +4360,6 @@ Jim_Cmd *Jim_GetCommand(Jim_Interp *interp, Jim_Obj *objPtr, int flags)
         Jim_IncrRefCount(interp->framePtr->nsObj);
         Jim_DecrRefCount(interp, qualifiedNameObj);
     }
-    else {
-        cmd = objPtr->internalRep.cmdValue.cmdPtr;
-    }
     while (cmd->u.proc.upcall) {
         cmd = cmd->prevCmd;
     }
@@ -5042,7 +5071,6 @@ static int JimDeleteLocalProcs(Jim_Interp *interp, Jim_Stack *localCommands)
                 else {
                     Jim_DeleteHashEntry(ht, cmdNameObj);
                 }
-                Jim_InterpIncrProcEpoch(interp);
             }
             Jim_DecrRefCount(interp, cmdNameObj);
         }
@@ -5624,6 +5652,8 @@ void Jim_FreeInterp(Jim_Interp *i)
 
     Jim_Obj *objPtr, *nextObjPtr;
 
+    i->quitting = 1;
+
     /* Free the active call frames list - must be done before i->commands is destroyed */
     for (cf = i->framePtr; cf; cf = cfx) {
         /* Note that we ignore any errors */
@@ -5643,6 +5673,9 @@ void Jim_FreeInterp(Jim_Interp *i)
     Jim_DecrRefCount(i, i->errorFileNameObj);
     Jim_DecrRefCount(i, i->currentScriptObj);
     Jim_DecrRefCount(i, i->nullScriptObj);
+
+    Jim_InterpIncrProcEpoch(i);
+
     Jim_FreeHashTable(&i->commands);
 #ifdef JIM_REFERENCES
     Jim_FreeHashTable(&i->references);
diff --git a/jim.h b/jim.h
index 6e9f9b3..ad66b16 100644
--- a/jim.h
+++ b/jim.h
@@ -526,6 +526,7 @@ typedef struct Jim_Interp {
                 'ID' field contained in the Jim_CallFrame
                 structure. */
     int local; /* If 'local' is in effect, newly defined procs keep a reference to the old defn */
+    int quitting; /* Set to 1 during Jim_FreeInterp() */
     Jim_Obj *liveList; /* Linked list of all the live objects. */
     Jim_Obj *freeList; /* Linked list of all the unused objects. */
     Jim_Obj *currentScriptObj; /* Script currently in execution. */
@@ -551,6 +552,8 @@ typedef struct Jim_Interp {
                   a command. It is set to what the user specified
                   via Jim_CreateCommand(). */
 
+    Jim_Cmd *oldCmdCache; /* commands that have been deleted, but may still be cached */
+    int oldCmdCacheSize; /* Number of delete commands */
     struct Jim_CallFrame *freeFramesList; /* list of CallFrame structures. */
     struct Jim_HashTable assocData; /* per-interp storage for use by packages */
     Jim_PrngState *prngState; /* per interpreter Random Number Gen. state. */
@@ -562,7 +565,6 @@ typedef struct Jim_Interp {
  * At some point may be a real function doing more work.
  * The proc epoch is used in order to know when a command lookup
  * cached can no longer considered valid. */
-#define Jim_InterpIncrProcEpoch(i) (i)->procEpoch++
 #define Jim_SetResultString(i,s,l) Jim_SetResult(i, Jim_NewStringObj(i,s,l))
 #define Jim_SetResultInt(i,intval) Jim_SetResult(i, Jim_NewIntObj(i,intval))
 /* Note: Using trueObj and falseObj here makes some things slower...*/