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;
 }