Use static list of descriptors to reduce mallocs.

Only if file descriptors with a value > 255 occur, the list
implementation is used.

diff --git a/src/coloredstderr.c b/src/coloredstderr.c
index ccefdf34a31c9da821a58e868008a303e5310c14..008da266bee76fbbc51efc7280edfe5dcf41fa47 100644 (file)
--- a/src/coloredstderr.c
+++ b/src/coloredstderr.c
@@ -66,7 +66,10 @@ static int check_handle_fd(int fd) {
     if (!initialized) {
         init_from_environment();
     }
-    if (tracked_fds_list_count == 0) {
+
+    /* tracked_fds_find() is most likely faster than calling isatty(),
+     * therefore check if we are tracking this file descriptor first. */
+    if (!tracked_fds_find(fd)) {
         return 0;
     }
 
@@ -76,7 +79,7 @@ static int check_handle_fd(int fd) {
         return 0;
     }
 
-    return tracked_fds_find(fd);
+    return 1;
 }
 
 static void dup_fd(int oldfd, int newfd) {
@@ -87,9 +90,6 @@ static void dup_fd(int oldfd, int newfd) {
     if (!initialized) {
         init_from_environment();
     }
-    if (tracked_fds_list_count == 0) {
-        return;
-    }
 
     /* We are already tracking this file descriptor, add newfd to the list as
      * it will reference the same descriptor. */
@@ -112,9 +112,6 @@ static void close_fd(int fd) {
     if (!initialized) {
         init_from_environment();
     }
-    if (tracked_fds_list_count == 0) {
-        return;
-    }
 
     tracked_fds_remove(fd);
 }

diff --git a/src/constants.h b/src/constants.h
index f7c9b77b1cde6b3cf18a55486f2d0d51bf53db06..66fe03feecc909f00c3baf5198ea10fad0f1b4c5 100644 (file)
--- a/src/constants.h
+++ b/src/constants.h
@@ -30,6 +30,10 @@
 #define DEFAULT_PRE_STRING "\e[91m"
 #define DEFAULT_POST_STRING "\e[0m"
 
+/* Number of elements to allocate statically. Highest descriptor observed in
+ * normal use was 255 (by bash), which yielded this limit to prevent
+ * unnecessary calls to malloc() whenever possible. */
+#define TRACKFDS_STATIC_COUNT 256
 /* Number of new elements to allocate per realloc(). */
 #define TRACKFDS_REALLOC_STEP 10
 

diff --git a/src/trackfds.h b/src/trackfds.h
index 67b956238d0a2cec08583b760ca792475ff56128..4571b2c9448fdc9d19aded63e222a10c8511c2e3 100644 (file)
--- a/src/trackfds.h
+++ b/src/trackfds.h
@@ -20,7 +20,11 @@
 #ifndef TRACKFDS_H
 #define TRACKFDS_H 1
 
-/* List of tracked file descriptors. */
+/* Array of tracked file descriptors. Used for fast lookups for the normally
+ * used file descriptors (0 <= fd < TRACKFDS_STATIC_COUNT). */
+static int tracked_fds[TRACKFDS_STATIC_COUNT];
+
+/* List of tracked file descriptors >= TRACKFDS_STATIC_COUNT. */
 static int *tracked_fds_list;
 /* Current number of items in the list. */
 static size_t tracked_fds_list_count;
@@ -30,16 +34,39 @@ static size_t tracked_fds_list_space;
 
 #ifdef DEBUG
 static void tracked_fds_debug(void) {
-    debug("    tracked_fds_list: %d/%d\t\t[%d]\n", tracked_fds_list_count,
-                                                   tracked_fds_list_space,
-                                                   getpid());
     size_t i;
+
+    for (i = 0; i < TRACKFDS_STATIC_COUNT; i++) {
+        if (tracked_fds[i]) {
+            debug("    tracked_fds[%d]: %d\n", i, tracked_fds[i]);
+        }
+    }
+    debug("    tracked_fds_list: %d/%d\t[%d]\n", tracked_fds_list_count,
+                                                 tracked_fds_list_space,
+                                                 getpid());
     for (i = 0; i < tracked_fds_list_count; i++) {
         debug("    tracked_fds_list[%d]: %d\n", i, tracked_fds_list[i]);
     }
 }
 #endif
 
+static int init_tracked_fds_list(size_t count) {
+    /* Reduce reallocs. */
+    count += TRACKFDS_REALLOC_STEP;
+
+    tracked_fds_list = malloc(count * sizeof(*tracked_fds_list));
+    if (!tracked_fds_list) {
+#ifdef DEBUG
+        warning("malloc(tracked_fds_list, %d) failed [%d]\n",
+                count * sizeof(*tracked_fds_list), getpid());
+#endif
+        return 0;
+    }
+
+    tracked_fds_list_space = count;
+    return 1;
+}
+
 /* Load tracked file descriptors from the environment. The environment is used
  * to pass the information to child processes.
  *
@@ -77,21 +104,11 @@ static void init_from_environment(void) {
             count++;
         }
     }
-    tracked_fds_list_space = count + TRACKFDS_REALLOC_STEP;
-
-    tracked_fds_list = malloc(tracked_fds_list_space * sizeof(*tracked_fds_list));
-    if (!tracked_fds_list) {
-#ifdef DEBUG
-        warning("malloc(%zu): failed [%d]\n",
-                tracked_fds_list_space * sizeof(*tracked_fds_list), getpid());
-#endif
-        return;
-    }
 
     size_t i = 0;
 
     /* Parse file descriptor numbers from environment string and store them as
-     * integers in tracked_fds_list. */
+     * integers in tracked_fds and tracked_fds_list. */
     char *last;
     for (x = env_copy, last = env_copy; *x; x++) {
         if (*x != ',') {
@@ -108,43 +125,76 @@ static void init_from_environment(void) {
         }
 
         *x = 0;
-        tracked_fds_list[i++] = atoi(last);
 
+        int fd = atoi(last);
+        if (fd < TRACKFDS_STATIC_COUNT) {
+            tracked_fds[fd] = 1;
+        } else {
+            if (!tracked_fds_list) {
+                /* Pessimistic count estimate, but allocating a few more
+                 * elements doesn't hurt. */
+                if (!init_tracked_fds_list(count)) {
+                    /* Couldn't allocate memory, skip this entry. */
+                    warning("foo\n");
+                    goto next;
+                }
+            }
+            tracked_fds_list[i++] = fd;
+#ifdef DEBUG
+            debug("  large fd: %d\n", fd);
+#endif
+        }
+
+next:
         last = x + 1;
     }
 
-    tracked_fds_list_count = count;
+    tracked_fds_list_count = i;
 
 #ifdef DEBUG
     tracked_fds_debug();
 #endif
 }
 
-static void update_environment_buffer(char *x) {
-    size_t i;
-    for (i = 0; i < tracked_fds_list_count; i++) {
-        int length = snprintf(x, 10 + 1, "%d", tracked_fds_list[i]);
-        if (length >= 10 + 1) {
-            /* Integer too big to fit the buffer, skip it. */
+static char *update_environment_buffer_entry(char *x, int fd) {
+    int length = snprintf(x, 10 + 1, "%d", fd);
+    if (length >= 10 + 1) {
+        /* Integer too big to fit the buffer, skip it. */
 #ifdef DEBUG
-            warning("update_environment_buffer_entry(): truncated fd: %d [%d]\n",
-                    tracked_fds_list[i], getpid());
+        warning("update_environment_buffer_entry(): truncated fd: %d [%d]\n",
+                fd, getpid());
 #endif
-            continue;
-        }
+        return x;
+    }
 
-        /* Write comma after number. */
-        x += length;
-        *x++ = ',';
-        /* Make sure the string is always zero terminated. */
-        *x = 0;
+    /* Write comma after number. */
+    x += length;
+    *x++ = ',';
+    /* Make sure the string is always zero terminated. */
+    *x = 0;
+
+    return x;
+}
+static void update_environment_buffer(char *x) {
+    size_t i;
+    for (i = 0; i < TRACKFDS_STATIC_COUNT; i++) {
+        if (tracked_fds[i]) {
+            x = update_environment_buffer_entry(x, (int)i);
+        }
+    }
+    for (i = 0; i < tracked_fds_list_count; i++) {
+        x = update_environment_buffer_entry(x, tracked_fds_list[i]);
     }
 }
 inline static size_t update_environment_buffer_size(void) {
-    /* An integer (32-bit) has at most 10 digits, + 1 for the comma after each
+    /* Use the maximum count (TRACKFDS_STATIC_COUNT) of used descriptors
+     * because it's simple and small enough not to be a problem.
+     *
+     * An integer (32-bit) has at most 10 digits, + 1 for the comma after each
      * number. Bigger file descriptors (which shouldn't occur in reality) are
      * skipped. */
-    return tracked_fds_list_count * (10 + 1) + 1 /* to fit '\0' */;
+    return (TRACKFDS_STATIC_COUNT + tracked_fds_list_count)
+               * (10 + 1) + 1 /* to fit '\0' */;
 }
 static void update_environment(void) {
 #ifdef DEBUG
@@ -172,6 +222,15 @@ static void update_environment(void) {
 
 
 static void tracked_fds_add(int fd) {
+    if (fd < TRACKFDS_STATIC_COUNT) {
+        tracked_fds[fd] = 1;
+#if 0
+        debug("tracked_fds_add(): %-3d\t\t[%d]\n", fd, getpid());
+        tracked_fds_debug();
+#endif
+        return;
+    }
+
     if (tracked_fds_list_count >= tracked_fds_list_space) {
         size_t new_space = tracked_fds_list_space + TRACKFDS_REALLOC_STEP;
         int *tmp = realloc(tracked_fds_list,
@@ -198,6 +257,17 @@ static void tracked_fds_add(int fd) {
 #endif
 }
 static int tracked_fds_remove(int fd) {
+    if (fd < TRACKFDS_STATIC_COUNT) {
+        int old_value = tracked_fds[fd];
+        tracked_fds[fd] = 0;
+
+#if 0
+        debug("tracked_fds_remove(): %-3d\t[%d]\n", fd, getpid());
+        tracked_fds_debug();
+#endif
+        return old_value; /* Found vs. not found. */
+    }
+
     size_t i;
     for (i = 0; i < tracked_fds_list_count; i++) {
         if (fd != tracked_fds_list[i]) {
@@ -221,6 +291,13 @@ static int tracked_fds_remove(int fd) {
     return 0;
 }
 static int tracked_fds_find(int fd) {
+    if (fd < TRACKFDS_STATIC_COUNT) {
+        return tracked_fds[fd];
+    }
+    if (tracked_fds_list_count == 0) {
+        return 0;
+    }
+
     size_t i;
     for (i = 0; i < tracked_fds_list_count; i++) {
         if (fd == tracked_fds_list[i]) {

diff --git a/tests/example.c b/tests/example.c
index c615211c11872600f005cd5cc3ecac98be3acc6c..b1eac98be9b6bc02cba8b80d3562f893e249e313 100644 (file)
--- a/tests/example.c
+++ b/tests/example.c
@@ -36,6 +36,17 @@ int main(int argc, char **argv) {
 
     fprintf(stderr, "\n");
     fprintf(stdout, "\n");
+    fflush(stdout);
+
+    /* Check usage of tracked_fds_list (at least in parts). No error checking
+     * here! */
+    dup2(STDERR_FILENO, 471);
+    dup2(471, 42);
+    write(471, "more on stderr\n", 15);
+    close(471);
+    dup2(STDOUT_FILENO, 471);
+    write(42, "stderr ...\n", 11);
+    write(471, "more on stdout\n", 15);
 
     return EXIT_SUCCESS;
 }

diff --git a/tests/example.expected b/tests/example.expected
index e55674833181ed2822d54ac26b7b80ada2c6aa14..edb9c584661e28f63458baa9ac172244ac54d074 100644 (file)
--- a/tests/example.expected
+++ b/tests/example.expected
@@ -3,3 +3,6 @@
 >STDERR>error!: Success
 <STDERR<>STDERR>write to stderr 2<STDERR<write to stdout 2>STDERR>
 <STDERR<
+>STDERR>more on stderr
+<STDERR<>STDERR>stderr ...
+<STDERR<more on stdout