apps.plugin optimization to eliminate several unneeded calls

diff --git a/src/apps_plugin.c b/src/apps_plugin.c
index 0a72190aaff7ccbc3cf1e794a997bd7c6416f7e4..20ebb3f846ca095c664ad5dcbe2701fcecef1faa 100644 (file)
--- a/src/apps_plugin.c
+++ b/src/apps_plugin.c
@@ -475,6 +475,8 @@ struct pid_stat {
     unsigned long long io_collected_usec;
     unsigned long long last_io_collected_usec;
 
+    char *fds_dirname;              // the full directory name in /proc/PID/fd
+
     char *stat_filename;
     char *statm_filename;
     char *io_filename;
@@ -522,11 +524,12 @@ static inline void del_pid_entry(pid_t pid) {
     if(all_pids[pid]->next) all_pids[pid]->next->prev = all_pids[pid]->prev;
     if(all_pids[pid]->prev) all_pids[pid]->prev->next = all_pids[pid]->next;
 
-    if(all_pids[pid]->fds) freez(all_pids[pid]->fds);
-    if(all_pids[pid]->stat_filename) freez(all_pids[pid]->stat_filename);
-    if(all_pids[pid]->statm_filename) freez(all_pids[pid]->statm_filename);
-    if(all_pids[pid]->io_filename) freez(all_pids[pid]->io_filename);
-    if(all_pids[pid]->cmdline_filename) freez(all_pids[pid]->cmdline_filename);
+    freez(all_pids[pid]->fds);
+    freez(all_pids[pid]->fds_dirname);
+    freez(all_pids[pid]->stat_filename);
+    freez(all_pids[pid]->statm_filename);
+    freez(all_pids[pid]->io_filename);
+    freez(all_pids[pid]->cmdline_filename);
     freez(all_pids[pid]);
 
     all_pids[pid] = NULL;
@@ -604,7 +607,8 @@ static inline int read_proc_pid_stat(struct pid_stat *p) {
     if(unlikely(!ff)) goto cleanup;
 
     // if(set_quotes) procfile_set_quotes(ff, "()");
-    if(set_quotes) procfile_set_open_close(ff, "(", ")");
+    if(unlikely(set_quotes))
+        procfile_set_open_close(ff, "(", ")");
 
     ff = procfile_readall(ff);
     if(unlikely(!ff)) goto cleanup;
@@ -615,7 +619,8 @@ static inline int read_proc_pid_stat(struct pid_stat *p) {
 
     // p->pid           = str2ul(procfile_lineword(ff, 0, 0+i));
 
-    strncpyz(p->comm, procfile_lineword(ff, 0, 1), MAX_COMPARE_NAME);
+    if(unlikely(!p->comm[0]))
+        strncpyz(p->comm, procfile_lineword(ff, 0, 1), MAX_COMPARE_NAME);
 
     // p->state         = *(procfile_lineword(ff, 0, 2));
     p->ppid             = (int32_t)str2ul(procfile_lineword(ff, 0, 3));
@@ -1172,74 +1177,106 @@ static inline int file_descriptor_find_or_add(const char *name)
     return file_descriptor_set_on_empty_slot(name, hash, type);
 }
 
+static inline void make_all_pid_fds_negative(struct pid_stat *p) {
+    int *fd = p->fds, *end = &p->fds[p->fds_size];
+    while(fd < end) {
+        *fd = -(*fd);
+        fd++;
+    }
+}
+
+static inline void cleanup_negative_pid_fds(struct pid_stat *p) {
+    int *fd = p->fds, *end = &p->fds[p->fds_size];
+    while(fd < end) {
+        if(unlikely(*fd < 0)) {
+            file_descriptor_not_used(-(*fd));
+            *fd++ = 0;
+        }
+        else
+            fd++;
+    }
+}
+
+static inline void zero_pid_fds(struct pid_stat *p, int first, int size) {
+    int *fd = &p->fds[first], *end = &p->fds[first + size];
+    while(fd < end) *fd++ = 0;
+}
+
 static inline int read_pid_file_descriptors(struct pid_stat *p) {
-    char dirname[FILENAME_MAX+1];
-
-    snprintfz(dirname, FILENAME_MAX, "%s/proc/%d/fd", global_host_prefix, p->pid);
-    DIR *fds = opendir(dirname);
-    if(fds) {
-        int c;
-        struct dirent *de;
-        char fdname[FILENAME_MAX + 1];
-        char linkname[FILENAME_MAX + 1];
-
-        // make the array negative
-        for(c = 0 ; c < p->fds_size ; c++)
-            p->fds[c] = -p->fds[c];
-
-        while((de = readdir(fds))) {
-            if(strcmp(de->d_name, ".") == 0 || strcmp(de->d_name, "..") == 0)
-                continue;
+    if(unlikely(!p->fds_dirname)) {
+        char dirname[FILENAME_MAX+1];
+        snprintfz(dirname, FILENAME_MAX, "%s/proc/%d/fd", global_host_prefix, p->pid);
+        p->fds_dirname = strdupz(dirname);
+    }
 
-            // check if the fds array is small
-            int fdid = (int)str2l(de->d_name);
-            if(fdid < 0) continue;
-            if(fdid >= p->fds_size) {
-                // it is small, extend it
-                if(unlikely(debug))
-                    fprintf(stderr, "apps.plugin: extending fd memory slots for %s from %d to %d\n", p->comm, p->fds_size, fdid + MAX_SPARE_FDS);
+    DIR *fds = opendir(p->fds_dirname);
+    if(unlikely(!fds)) return 0;
 
-                p->fds = reallocz(p->fds, (fdid + MAX_SPARE_FDS) * sizeof(int));
+    struct dirent *de;
+    char fdname[FILENAME_MAX + 1];
+    char linkname[FILENAME_MAX + 1];
 
-                // and initialize it
-                for(c = p->fds_size ; c < (fdid + MAX_SPARE_FDS) ; c++) p->fds[c] = 0;
-                p->fds_size = fdid + MAX_SPARE_FDS;
-            }
+    // we make all pid fds negative, so that
+    // we can detect unused file descriptors
+    // at the end, to free them
+    make_all_pid_fds_negative(p);
+
+    while((de = readdir(fds))) {
+        // we need only files with numeric names
+
+        if(unlikely(de->d_name[0] < '0' || de->d_name[0] > '9'))
+            continue;
 
-            if(p->fds[fdid] == 0) {
-                // we don't know this fd, get it
-
-                sprintf(fdname, "%s/proc/%d/fd/%s", global_host_prefix, p->pid, de->d_name);
-                ssize_t l = readlink(fdname, linkname, FILENAME_MAX);
-                if(l == -1) {
-                    if(debug || (p->target && p->target->debug)) {
-                        if(debug || (p->target && p->target->debug))
-                            error("Cannot read link %s", fdname);
-                    }
-                    continue;
+        // get its number
+        int fdid = (int)str2l(de->d_name);
+        if(unlikely(fdid < 0)) continue;
+
+        // check if the fds array is small
+        if(unlikely(fdid >= p->fds_size)) {
+            // it is small, extend it
+
+            if(unlikely(debug))
+                fprintf(stderr, "apps.plugin: extending fd memory slots for %s from %d to %d\n", p->comm, p->fds_size, fdid + MAX_SPARE_FDS);
+
+            p->fds = reallocz(p->fds, (fdid + MAX_SPARE_FDS) * sizeof(int));
+
+            // and initialize it
+            zero_pid_fds(p, p->fds_size, (fdid + MAX_SPARE_FDS) - p->fds_size);
+            p->fds_size = fdid + MAX_SPARE_FDS;
+        }
+
+        if(unlikely(p->fds[fdid] == 0)) {
+            // we don't know this fd, get it
+
+            sprintf(fdname, "%s/proc/%d/fd/%s", global_host_prefix, p->pid, de->d_name);
+            ssize_t l = readlink(fdname, linkname, FILENAME_MAX);
+            if(unlikely(l == -1)) {
+                if(debug || (p->target && p->target->debug)) {
+                    if(debug || (p->target && p->target->debug))
+                        error("Cannot read link %s", fdname);
                 }
+                continue;
+            }
+            else
                 linkname[l] = '\0';
-                file_counter++;
 
-                // if another process already has this, we will get
-                // the same id
-                p->fds[fdid] = file_descriptor_find_or_add(linkname);
-            }
+            file_counter++;
 
-            // else make it positive again, we need it
-            // of course, the actual file may have changed, but we don't care so much
-            // FIXME: we could compare the inode as returned by readdir dirent structure
-            else p->fds[fdid] = -p->fds[fdid];
+            // if another process already has this, we will get
+            // the same id
+            p->fds[fdid] = file_descriptor_find_or_add(linkname);
         }
-        closedir(fds);
 
-        // remove all the negative file descriptors
-        for(c = 0 ; c < p->fds_size ; c++) if(p->fds[c] < 0) {
-            file_descriptor_not_used(-p->fds[c]);
-            p->fds[c] = 0;
-        }
+        // else make it positive again, we need it
+        // of course, the actual file may have changed, but we don't care so much
+        // FIXME: we could compare the inode as returned by readdir dirent structure
+
+        else
+            p->fds[fdid] = -p->fds[fdid];
     }
-    else return 0;
+
+    closedir(fds);
+    cleanup_negative_pid_fds(p);
 
     return 1;
 }
@@ -1594,6 +1631,37 @@ static inline int managed_log(struct pid_stat *p, uint32_t log, int status) {
     return status;
 }
 
+static inline void assign_target_to_pid(struct pid_stat *p) {
+    uint32_t hash = simple_hash(p->comm);
+    size_t pclen  = strlen(p->comm);
+
+    struct target *w;
+    for(w = apps_groups_root_target; w ; w = w->next) {
+        // if(debug || (p->target && p->target->debug)) fprintf(stderr, "apps.plugin: \t\tcomparing '%s' with '%s'\n", w->compare, p->comm);
+
+        // find it - 4 cases:
+        // 1. the target is not a pattern
+        // 2. the target has the prefix
+        // 3. the target has the suffix
+        // 4. the target is something inside cmdline
+
+        if(unlikely(( (!w->starts_with && !w->ends_with && w->comparehash == hash && !strcmp(w->compare, p->comm))
+            || (w->starts_with && !w->ends_with && !strncmp(w->compare, p->comm, w->comparelen))
+            || (!w->starts_with && w->ends_with && pclen >= w->comparelen && !strcmp(w->compare, &p->comm[pclen - w->comparelen]))
+            || (proc_pid_cmdline_is_needed && w->starts_with && w->ends_with && strstr(p->cmdline, w->compare))
+                ))) {
+
+            if(w->target) p->target = w->target;
+            else p->target = w;
+
+            if(debug || (p->target && p->target->debug))
+                fprintf(stderr, "apps.plugin: \t\t%s linked to target %s\n", p->comm, p->target->name);
+
+            break;
+        }
+    }
+}
+
 static inline int collect_data_for_pid(pid_t pid) {
     if(unlikely(pid <= 0 || pid > pid_max)) {
         error("Invalid pid %d read (expected 1 to %d). Ignoring process.", pid, pid_max);
@@ -1646,32 +1714,7 @@ static inline int collect_data_for_pid(pid_t pid) {
         if(unlikely(debug))
             fprintf(stderr, "apps.plugin: \tJust added %d (%s)\n", pid, p->comm);
 
-        uint32_t hash = simple_hash(p->comm);
-        size_t pclen  = strlen(p->comm);
-
-        struct target *w;
-        for(w = apps_groups_root_target; w ; w = w->next) {
-            // if(debug || (p->target && p->target->debug)) fprintf(stderr, "apps.plugin: \t\tcomparing '%s' with '%s'\n", w->compare, p->comm);
-
-            // find it - 4 cases:
-            // 1. the target is not a pattern
-            // 2. the target has the prefix
-            // 3. the target has the suffix
-            // 4. the target is something inside cmdline
-            if( (!w->starts_with && !w->ends_with && w->comparehash == hash && !strcmp(w->compare, p->comm))
-                   || (w->starts_with && !w->ends_with && !strncmp(w->compare, p->comm, w->comparelen))
-                   || (!w->starts_with && w->ends_with && pclen >= w->comparelen && !strcmp(w->compare, &p->comm[pclen - w->comparelen]))
-                   || (proc_pid_cmdline_is_needed && w->starts_with && w->ends_with && strstr(p->cmdline, w->compare))
-                    ) {
-                if(w->target) p->target = w->target;
-                else p->target = w;
-
-                if(debug || (p->target && p->target->debug))
-                    fprintf(stderr, "apps.plugin: \t\t%s linked to target %s\n", p->comm, p->target->name);
-
-                break;
-            }
-        }
+        assign_target_to_pid(p);
     }
 
     // --------------------------------------------------------------------

diff --git a/src/procfile.c b/src/procfile.c
index 6f52bf465fb28dba7f4aaba00db84d101c03f014..2281c02bc4f8910718b61486071a93f91cda0272 100644 (file)
--- a/src/procfile.c
+++ b/src/procfile.c
@@ -15,6 +15,25 @@ size_t procfile_max_lines = PFLINES_INCREASE_STEP;
 size_t procfile_max_words = PFWORDS_INCREASE_STEP;
 size_t procfile_max_allocation = PROCFILE_INCREMENT_BUFFER;
 
+
+// ----------------------------------------------------------------------------
+
+char *procfile_filename(procfile *ff) {
+    char buffer[FILENAME_MAX + 1];
+    snprintfz(buffer, FILENAME_MAX, "/proc/self/fd/%d", ff->fd);
+
+    ssize_t l = readlink(buffer, ff->filename, FILENAME_MAX);
+    if(unlikely(l == -1))
+        snprintfz(ff->filename, FILENAME_MAX, "unknown filename for fd %d", ff->fd);
+    else
+        ff->filename[l] = '\0';
+
+    // on non-linux systems, something like this will be needed
+    // fcntl(ff->fd, F_GETPATH, ff->filename)
+
+    return ff->filename;
+}
+
 // ----------------------------------------------------------------------------
 // An array of words
 
@@ -114,7 +133,7 @@ static inline void pflines_free(pflines *fl) {
 #define PF_CHAR_IS_CLOSE        'C'
 
 void procfile_close(procfile *ff) {
-    debug(D_PROCFILE, PF_PREFIX ": Closing file '%s'", ff->filename);
+    debug(D_PROCFILE, PF_PREFIX ": Closing file '%s'", procfile_filename(ff));
 
     if(likely(ff->lines)) pflines_free(ff->lines);
     if(likely(ff->words)) pfwords_free(ff->words);
@@ -250,26 +269,24 @@ static inline void procfile_parser(procfile *ff) {
 }
 
 procfile *procfile_readall(procfile *ff) {
-    debug(D_PROCFILE, PF_PREFIX ": Reading file '%s'.", ff->filename);
-
-    ssize_t r = 1;
-    ff->len = 0;
+    // debug(D_PROCFILE, PF_PREFIX ": Reading file '%s'.", ff->filename);
 
-    while(likely(r > 0)) {
+    ff->len = 0;    // zero the used size
+    ssize_t r = 1;  // read at least once
+    while(r > 0) {
         ssize_t s = ff->len;
         ssize_t x = ff->size - s;
 
         if(unlikely(!x)) {
-            debug(D_PROCFILE, PF_PREFIX ": Expanding data buffer for file '%s'.", ff->filename);
-
+            debug(D_PROCFILE, PF_PREFIX ": Expanding data buffer for file '%s'.", procfile_filename(ff));
             ff = reallocz(ff, sizeof(procfile) + ff->size + PROCFILE_INCREMENT_BUFFER);
             ff->size += PROCFILE_INCREMENT_BUFFER;
         }
 
-        debug(D_PROCFILE, "Reading file '%s', from position %ld with length %lu", ff->filename, s, ff->size - s);
+        debug(D_PROCFILE, "Reading file '%s', from position %ld with length %lu", procfile_filename(ff), s, ff->size - s);
         r = read(ff->fd, &ff->data[s], ff->size - s);
         if(unlikely(r == -1)) {
-            if(unlikely(!(ff->flags & PROCFILE_FLAG_NO_ERROR_ON_FILE_IO))) error(PF_PREFIX ": Cannot read from file '%s'", ff->filename);
+            if(unlikely(!(ff->flags & PROCFILE_FLAG_NO_ERROR_ON_FILE_IO))) error(PF_PREFIX ": Cannot read from file '%s'", procfile_filename(ff));
             procfile_close(ff);
             return NULL;
         }
@@ -277,9 +294,9 @@ procfile *procfile_readall(procfile *ff) {
         ff->len += r;
     }
 
-    debug(D_PROCFILE, "Rewinding file '%s'", ff->filename);
+    // debug(D_PROCFILE, "Rewinding file '%s'", ff->filename);
     if(unlikely(lseek(ff->fd, 0, SEEK_SET) == -1)) {
-        if(unlikely(!(ff->flags & PROCFILE_FLAG_NO_ERROR_ON_FILE_IO))) error(PF_PREFIX ": Cannot rewind on file '%s'.", ff->filename);
+        if(unlikely(!(ff->flags & PROCFILE_FLAG_NO_ERROR_ON_FILE_IO))) error(PF_PREFIX ": Cannot rewind on file '%s'.", procfile_filename(ff));
         procfile_close(ff);
         return NULL;
     }
@@ -294,7 +311,7 @@ procfile *procfile_readall(procfile *ff) {
         if(unlikely(ff->words->len > procfile_max_words)) procfile_max_words = ff->words->len;
     }
 
-    debug(D_PROCFILE, "File '%s' updated.", ff->filename);
+    // debug(D_PROCFILE, "File '%s' updated.", ff->filename);
     return ff;
 }
 
@@ -379,7 +396,8 @@ procfile *procfile_open(const char *filename, const char *separators, uint32_t f
 
     size_t size = (unlikely(procfile_adaptive_initial_allocation)) ? procfile_max_allocation : PROCFILE_INCREMENT_BUFFER;
     procfile *ff = mallocz(sizeof(procfile) + size);
-    strncpyz(ff->filename, filename, FILENAME_MAX);
+
+    //strncpyz(ff->filename, filename, FILENAME_MAX);
 
     ff->fd = fd;
     ff->size = size;
@@ -406,7 +424,7 @@ procfile *procfile_reopen(procfile *ff, const char *filename, const char *separa
         return NULL;
     }
 
-    strncpyz(ff->filename, filename, FILENAME_MAX);
+    //strncpyz(ff->filename, filename, FILENAME_MAX);
 
     ff->flags = flags;
 
@@ -423,7 +441,7 @@ void procfile_print(procfile *ff) {
     size_t lines = procfile_lines(ff), l;
     char *s;
 
-    debug(D_PROCFILE, "File '%s' with %zu lines and %zu words", ff->filename, ff->lines->len, ff->words->len);
+    debug(D_PROCFILE, "File '%s' with %zu lines and %zu words", procfile_filename(ff), ff->lines->len, ff->words->len);
 
     for(l = 0; likely(l < lines) ;l++) {
         size_t words = procfile_linewords(ff, l);

diff --git a/src/procfile.h b/src/procfile.h
index a586ba48d461ef617ebc4e4cc0012371a9c1f644..dae5a0fc272092deaea95dd565392ae77a9ee5b0 100644 (file)
--- a/src/procfile.h
+++ b/src/procfile.h
@@ -59,7 +59,8 @@ typedef struct {
 #define PROCFILE_FLAG_NO_ERROR_ON_FILE_IO 0x00000001
 
 typedef struct {
-    char filename[FILENAME_MAX + 1];
+    char filename[FILENAME_MAX + 1]; // not populated until profile_filename() is called
+
     uint32_t flags;
     int fd;               // the file desriptor
     size_t len;           // the bytes we have placed into data
@@ -89,6 +90,8 @@ extern void procfile_print(procfile *ff);
 extern void procfile_set_quotes(procfile *ff, const char *quotes);
 extern void procfile_set_open_close(procfile *ff, const char *open, const char *close);
 
+extern char *procfile_filename(procfile *ff);
+
 // ----------------------------------------------------------------------------
 
 // set this to 1, to have procfile adapt its initial buffer allocation to the max allocation used so far