Improve some ancient, crufty code in bootstrap + initdb.

At some point back in the last century, somebody felt that reading
all of pg_type twice was cheaper, or at least easier, than using
repalloc() to resize the Typ[] array dynamically.  That seems like an
entirely wacko proposition, so rewrite the code to do it the other
way.  (To add insult to injury, there were two not-quite-identical
copies of said code.)

initdb.c's readfile() function had the same disease of preferring
to do double the I/O to avoid resizing its output array.  Here,
we can make things easier by using the just-invented pg_get_line()
function to handle reading individual lines without a predetermined
notion of how long they are.

On my machine, it's difficult to detect any net change in the
overall runtime of initdb from these changes; but they should
help on slower buildfarm machines (especially since a buildfarm
cycle involves a lot of initdb's these days).

My attention was drawn to these places by scan-build complaints,
but on inspection they needed a lot more work than just suppressing
dead stores :-(

diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c
index 45b7efbe4659f8cb5b824f9b414e75e614be8b50..76b2f5066f680cb448dfc2c2a472dd6e1f6d3b1a 100644 (file)
--- a/src/backend/bootstrap/bootstrap.c
+++ b/src/backend/bootstrap/bootstrap.c
@@ -53,14 +53,12 @@
 uint32     bootstrap_data_checksum_version = 0;    /* No checksum */
 
 
-#define ALLOC(t, c) \
-   ((t *) MemoryContextAllocZero(TopMemoryContext, (unsigned)(c) * sizeof(t)))
-
 static void CheckerModeMain(void);
 static void BootstrapModeMain(void);
 static void bootstrap_signals(void);
 static void ShutdownAuxiliaryProcess(int code, Datum arg);
 static Form_pg_attribute AllocateAttribute(void);
+static void populate_typ_array(void);
 static Oid gettype(char *type);
 static void cleanup(void);
 
@@ -583,46 +581,24 @@ ShutdownAuxiliaryProcess(int code, Datum arg)
 
 /* ----------------
  *     boot_openrel
+ *
+ * Execute BKI OPEN command.
  * ----------------
  */
 void
 boot_openrel(char *relname)
 {
    int         i;
-   struct typmap **app;
-   Relation    rel;
-   TableScanDesc scan;
-   HeapTuple   tup;
 
    if (strlen(relname) >= NAMEDATALEN)
        relname[NAMEDATALEN - 1] = '\0';
 
+   /*
+    * pg_type must be filled before any OPEN command is executed, hence we
+    * can now populate the Typ array if we haven't yet.
+    */
    if (Typ == NULL)
-   {
-       /* We can now load the pg_type data */
-       rel = table_open(TypeRelationId, NoLock);
-       scan = table_beginscan_catalog(rel, 0, NULL);
-       i = 0;
-       while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
-           ++i;
-       table_endscan(scan);
-       app = Typ = ALLOC(struct typmap *, i + 1);
-       while (i-- > 0)
-           *app++ = ALLOC(struct typmap, 1);
-       *app = NULL;
-       scan = table_beginscan_catalog(rel, 0, NULL);
-       app = Typ;
-       while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
-       {
-           (*app)->am_oid = ((Form_pg_type) GETSTRUCT(tup))->oid;
-           memcpy((char *) &(*app)->am_typ,
-                  (char *) GETSTRUCT(tup),
-                  sizeof((*app)->am_typ));
-           app++;
-       }
-       table_endscan(scan);
-       table_close(rel, NoLock);
-   }
+       populate_typ_array();
 
    if (boot_reldesc != NULL)
        closerel(NULL);
@@ -889,6 +865,52 @@ cleanup(void)
        closerel(NULL);
 }
 
+/* ----------------
+ *     populate_typ_array
+ *
+ * Load the Typ array by reading pg_type.
+ * ----------------
+ */
+static void
+populate_typ_array(void)
+{
+   Relation    rel;
+   TableScanDesc scan;
+   HeapTuple   tup;
+   int         nalloc;
+   int         i;
+
+   Assert(Typ == NULL);
+
+   nalloc = 512;
+   Typ = (struct typmap **)
+       MemoryContextAlloc(TopMemoryContext, nalloc * sizeof(struct typmap *));
+
+   rel = table_open(TypeRelationId, NoLock);
+   scan = table_beginscan_catalog(rel, 0, NULL);
+   i = 0;
+   while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
+   {
+       Form_pg_type typForm = (Form_pg_type) GETSTRUCT(tup);
+
+       /* make sure there will be room for a trailing NULL pointer */
+       if (i >= nalloc - 1)
+       {
+           nalloc *= 2;
+           Typ = (struct typmap **)
+               repalloc(Typ, nalloc * sizeof(struct typmap *));
+       }
+       Typ[i] = (struct typmap *)
+           MemoryContextAlloc(TopMemoryContext, sizeof(struct typmap));
+       Typ[i]->am_oid = typForm->oid;
+       memcpy(&(Typ[i]->am_typ), typForm, sizeof(Typ[i]->am_typ));
+       i++;
+   }
+   Typ[i] = NULL;              /* Fill trailing NULL pointer */
+   table_endscan(scan);
+   table_close(rel, NoLock);
+}
+
 /* ----------------
  *     gettype
  *
@@ -903,14 +925,10 @@ cleanup(void)
 static Oid
 gettype(char *type)
 {
-   int         i;
-   Relation    rel;
-   TableScanDesc scan;
-   HeapTuple   tup;
-   struct typmap **app;
-
    if (Typ != NULL)
    {
+       struct typmap **app;
+
        for (app = Typ; *app != NULL; app++)
        {
            if (strncmp(NameStr((*app)->am_typ.typname), type, NAMEDATALEN) == 0)
@@ -922,33 +940,16 @@ gettype(char *type)
    }
    else
    {
+       int         i;
+
        for (i = 0; i < n_types; i++)
        {
            if (strncmp(type, TypInfo[i].name, NAMEDATALEN) == 0)
                return i;
        }
+       /* Not in TypInfo, so we'd better be able to read pg_type now */
        elog(DEBUG4, "external type: %s", type);
-       rel = table_open(TypeRelationId, NoLock);
-       scan = table_beginscan_catalog(rel, 0, NULL);
-       i = 0;
-       while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
-           ++i;
-       table_endscan(scan);
-       app = Typ = ALLOC(struct typmap *, i + 1);
-       while (i-- > 0)
-           *app++ = ALLOC(struct typmap, 1);
-       *app = NULL;
-       scan = table_beginscan_catalog(rel, 0, NULL);
-       app = Typ;
-       while ((tup = heap_getnext(scan, ForwardScanDirection)) != NULL)
-       {
-           (*app)->am_oid = ((Form_pg_type) GETSTRUCT(tup))->oid;
-           memmove((char *) &(*app++)->am_typ,
-                   (char *) GETSTRUCT(tup),
-                   sizeof((*app)->am_typ));
-       }
-       table_endscan(scan);
-       table_close(rel, NoLock);
+       populate_typ_array();
        return gettype(type);
    }
    elog(ERROR, "unrecognized type \"%s\"", type);

diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 73ddf408654a8c0b7bb392bf1bb4bb4f6ac632be..861b8817b9311e4e0962307c725c47c1f076ae9d 100644 (file)
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -468,14 +468,11 @@ filter_lines_with_token(char **lines, const char *token)
 static char **
 readfile(const char *path)
 {
+   char      **result;
    FILE       *infile;
-   int         maxlength = 1,
-               linelen = 0;
-   int         nlines = 0;
+   int         maxlines;
    int         n;
-   char      **result;
-   char       *buffer;
-   int         c;
+   char       *ln;
 
    if ((infile = fopen(path, "r")) == NULL)
    {
@@ -483,39 +480,24 @@ readfile(const char *path)
        exit(1);
    }
 
-   /* pass over the file twice - the first time to size the result */
+   maxlines = 1024;
+   result = (char **) pg_malloc(maxlines * sizeof(char *));
 
-   while ((c = fgetc(infile)) != EOF)
+   n = 0;
+   while ((ln = pg_get_line(infile)) != NULL)
    {
-       linelen++;
-       if (c == '\n')
+       /* make sure there will be room for a trailing NULL pointer */
+       if (n >= maxlines - 1)
        {
-           nlines++;
-           if (linelen > maxlength)
-               maxlength = linelen;
-           linelen = 0;
+           maxlines *= 2;
+           result = (char **) pg_realloc(result, maxlines * sizeof(char *));
        }
-   }
-
-   /* handle last line without a terminating newline (yuck) */
-   if (linelen)
-       nlines++;
-   if (linelen > maxlength)
-       maxlength = linelen;
 
-   /* set up the result and the line buffer */
-   result = (char **) pg_malloc((nlines + 1) * sizeof(char *));
-   buffer = (char *) pg_malloc(maxlength + 1);
-
-   /* now reprocess the file and store the lines */
-   rewind(infile);
-   n = 0;
-   while (fgets(buffer, maxlength + 1, infile) != NULL && n < nlines)
-       result[n++] = pg_strdup(buffer);
+       result[n++] = ln;
+   }
+   result[n] = NULL;
 
    fclose(infile);
-   free(buffer);
-   result[n] = NULL;
 
    return result;
 }