Clean up temporary-memory management during ispell dictionary loading.

Add explicit initialization and cleanup functions to spell.c, and keep
all working state in the already-existing ISpellDict struct.  This lets us
get rid of a static variable along with some extremely shaky assumptions
about usage of child memory contexts.

This commit is just code beautification and has no impact on functionality
or performance, but it opens the way to a less-grotty implementation of
Pavel's memory-saving hack, which will follow shortly.

diff --git a/src/backend/tsearch/dict_ispell.c b/src/backend/tsearch/dict_ispell.c
index 95bf8d46324adb961c8dbe130df867eb94b04f80..720adb0c1abe9373093ed59c601362d64841bdd4 100644 (file)
--- a/src/backend/tsearch/dict_ispell.c
+++ b/src/backend/tsearch/dict_ispell.c
@@ -19,7 +19,6 @@
 #include "tsearch/ts_public.h"
 #include "tsearch/ts_utils.h"
 #include "utils/builtins.h"
-#include "utils/memutils.h"
 
 
 typedef struct
@@ -40,6 +39,8 @@ dispell_init(PG_FUNCTION_ARGS)
 
    d = (DictISpell *) palloc0(sizeof(DictISpell));
 
+   NIStartBuild(&(d->obj));
+
    foreach(l, dictoptions)
    {
        DefElem    *defel = (DefElem *) lfirst(l);
@@ -102,7 +103,7 @@ dispell_init(PG_FUNCTION_ARGS)
                 errmsg("missing DictFile parameter")));
    }
 
-   MemoryContextDeleteChildren(CurrentMemoryContext);
+   NIFinishBuild(&(d->obj));
 
    PG_RETURN_POINTER(d);
 }

diff --git a/src/backend/tsearch/spell.c b/src/backend/tsearch/spell.c
index 4b54b158f3e34901d65b86d8318364b382a8cfdc..cd7ada6613872e65cd00e276faf120ac0b470d71 100644 (file)
--- a/src/backend/tsearch/spell.c
+++ b/src/backend/tsearch/spell.c
@@ -21,42 +21,57 @@
 
 /*
  * Initialization requires a lot of memory that's not needed
- * after the initialization is done.  In init function,
- * CurrentMemoryContext is a long lived memory context associated
- * with the dictionary cache entry, so we use a temporary context
- * for the short-lived stuff.
+ * after the initialization is done.  During initialization,
+ * CurrentMemoryContext is the long-lived memory context associated
+ * with the dictionary cache entry.  We keep the short-lived stuff
+ * in the Conf->buildCxt context.
  */
-static MemoryContext tmpCtx = NULL;
+#define tmpalloc(sz)  MemoryContextAlloc(Conf->buildCxt, (sz))
+#define tmpalloc0(sz)  MemoryContextAllocZero(Conf->buildCxt, (sz))
 
-#define tmpalloc(sz)  MemoryContextAlloc(tmpCtx, (sz))
-#define tmpalloc0(sz)  MemoryContextAllocZero(tmpCtx, (sz))
-
-static void
-checkTmpCtx(void)
+/*
+ * Prepare for constructing an ISpell dictionary.
+ *
+ * The IspellDict struct is assumed to be zeroed when allocated.
+ */
+void
+NIStartBuild(IspellDict *Conf)
 {
    /*
-    * XXX: This assumes that CurrentMemoryContext doesn't have any children
-    * other than the one we create here.
+    * The temp context is a child of CurTransactionContext, so that it will
+    * go away automatically on error.
     */
-   if (CurrentMemoryContext->firstchild == NULL)
-   {
-       tmpCtx = AllocSetContextCreate(CurrentMemoryContext,
-                                      "Ispell dictionary init context",
-                                      ALLOCSET_DEFAULT_MINSIZE,
-                                      ALLOCSET_DEFAULT_INITSIZE,
-                                      ALLOCSET_DEFAULT_MAXSIZE);
-   }
-   else
-       tmpCtx = CurrentMemoryContext->firstchild;
+   Conf->buildCxt = AllocSetContextCreate(CurTransactionContext,
+                                          "Ispell dictionary init context",
+                                          ALLOCSET_DEFAULT_MINSIZE,
+                                          ALLOCSET_DEFAULT_INITSIZE,
+                                          ALLOCSET_DEFAULT_MAXSIZE);
 }
 
+/*
+ * Clean up when dictionary construction is complete.
+ */
+void
+NIFinishBuild(IspellDict *Conf)
+{
+   /* Release no-longer-needed temp memory */
+   MemoryContextDelete(Conf->buildCxt);
+   /* Just for cleanliness, zero the now-dangling pointers */
+   Conf->buildCxt = NULL;
+   Conf->Spell = NULL;
+}
+
+
+/*
+ * Apply lowerstr(), producing a temporary result (in the buildCxt).
+ */
 static char *
-lowerstr_ctx(char *src)
+lowerstr_ctx(IspellDict *Conf, const char *src)
 {
    MemoryContext saveCtx;
    char       *dst;
 
-   saveCtx = MemoryContextSwitchTo(tmpCtx);
+   saveCtx = MemoryContextSwitchTo(Conf->buildCxt);
    dst = lowerstr(src);
    MemoryContextSwitchTo(saveCtx);
 
@@ -120,6 +135,7 @@ strbcmp(const unsigned char *s1, const unsigned char *s2)
 
    return 0;
 }
+
 static int
 strbncmp(const unsigned char *s1, const unsigned char *s2, size_t count)
 {
@@ -196,8 +212,6 @@ NIImportDictionary(IspellDict *Conf, const char *filename)
    tsearch_readline_state trst;
    char       *line;
 
-   checkTmpCtx();
-
    if (!tsearch_readline_begin(&trst, filename))
        ereport(ERROR,
                (errcode(ERRCODE_CONFIG_FILE_ERROR),
@@ -242,7 +256,7 @@ NIImportDictionary(IspellDict *Conf, const char *filename)
            }
            s += pg_mblen(s);
        }
-       pstr = lowerstr_ctx(line);
+       pstr = lowerstr_ctx(Conf, line);
 
        NIAddSpell(Conf, pstr, flag);
        pfree(pstr);
@@ -545,8 +559,6 @@ NIImportOOAffixes(IspellDict *Conf, const char *filename)
    char        scanbuf[BUFSIZ];
    char       *recoded;
 
-   checkTmpCtx();
-
    /* read file to find any flag */
    memset(Conf->flagval, 0, sizeof(Conf->flagval));
    Conf->usecompound = false;
@@ -624,7 +636,7 @@ NIImportOOAffixes(IspellDict *Conf, const char *filename)
 
        if (ptype)
            pfree(ptype);
-       ptype = lowerstr_ctx(type);
+       ptype = lowerstr_ctx(Conf, type);
        if (scanread < 4 || (STRNCMP(ptype, "sfx") && STRNCMP(ptype, "pfx")))
            goto nextline;
 
@@ -646,7 +658,7 @@ NIImportOOAffixes(IspellDict *Conf, const char *filename)
 
            if (strlen(sflag) != 1 || flag != *sflag || flag == 0)
                goto nextline;
-           prepl = lowerstr_ctx(repl);
+           prepl = lowerstr_ctx(Conf, repl);
            /* affix flag */
            if ((ptr = strchr(prepl, '/')) != NULL)
            {
@@ -658,8 +670,8 @@ NIImportOOAffixes(IspellDict *Conf, const char *filename)
                    ptr++;
                }
            }
-           pfind = lowerstr_ctx(find);
-           pmask = lowerstr_ctx(mask);
+           pfind = lowerstr_ctx(Conf, find);
+           pmask = lowerstr_ctx(Conf, mask);
            if (t_iseq(find, '0'))
                *pfind = '\0';
            if (t_iseq(repl, '0'))
@@ -702,8 +714,6 @@ NIImportAffixes(IspellDict *Conf, const char *filename)
    bool        oldformat = false;
    char       *recoded = NULL;
 
-   checkTmpCtx();
-
    if (!tsearch_readline_begin(&trst, filename))
        ereport(ERROR,
                (errcode(ERRCODE_CONFIG_FILE_ERROR),
@@ -945,8 +955,6 @@ NISortDictionary(IspellDict *Conf)
    int         naffix = 0;
    int         curaffix;
 
-   checkTmpCtx();
-
    /* compress affixes */
 
    /* Count the number of different flags used in the dictionary */
@@ -985,8 +993,6 @@ NISortDictionary(IspellDict *Conf)
 
    qsort((void *) Conf->Spell, Conf->nspell, sizeof(SPELL *), cmpspell);
    Conf->Dictionary = mkSPNode(Conf, 0, Conf->nspell, 0);
-
-   Conf->Spell = NULL;
 }
 
 static AffixNode *
@@ -1123,8 +1129,6 @@ NISortAffixes(IspellDict *Conf)
    CMPDAffix  *ptr;
    int         firstsuffix = Conf->naffixes;
 
-   checkTmpCtx();
-
    if (Conf->naffixes == 0)
        return;
 

diff --git a/src/include/tsearch/dicts/spell.h b/src/include/tsearch/dicts/spell.h
index c2751c1690a0020e5df0aa6ddbca3399a7cb7e8e..421a636dc717e9217bcadedd5bc2ab5421e87ea2 100644 (file)
--- a/src/include/tsearch/dicts/spell.h
+++ b/src/include/tsearch/dicts/spell.h
@@ -138,14 +138,6 @@ typedef struct
    int         naffixes;
    AFFIX      *Affix;
 
-   /*
-    * Temporary array of all words in the dict file. Only used during
-    * initialization
-    */
-   SPELL     **Spell;
-   int         nspell;         /* number of valid entries in Spell array */
-   int         mspell;         /* allocated length of Spell array */
-
    AffixNode  *Suffix;
    AffixNode  *Prefix;
 
@@ -158,12 +150,26 @@ typedef struct
 
    unsigned char flagval[256];
    bool        usecompound;
+
+   /*
+    * Remaining fields are only used during dictionary construction;
+    * they are set up by NIStartBuild and cleared by NIFinishBuild.
+    */
+   MemoryContext   buildCxt;   /* temp context for construction */
+
+   /* Temporary array of all words in the dict file */
+   SPELL     **Spell;
+   int         nspell;         /* number of valid entries in Spell array */
+   int         mspell;         /* allocated length of Spell array */
 } IspellDict;
 
 extern TSLexeme *NINormalizeWord(IspellDict *Conf, char *word);
+
+extern void NIStartBuild(IspellDict *Conf);
 extern void NIImportAffixes(IspellDict *Conf, const char *filename);
 extern void NIImportDictionary(IspellDict *Conf, const char *filename);
 extern void NISortDictionary(IspellDict *Conf);
 extern void NISortAffixes(IspellDict *Conf);
+extern void NIFinishBuild(IspellDict *Conf);
 
 #endif