optimize workers statistics performance (#14077)

* find the issue with workers performance and optimize it

* have 2 independent locks in workers

* break down workers statistics work to measure the time of each module

* optimize reading cpu for workers; cleanup procfile from unecessary allocations and operations

* cleanup workers

* fix the workers statistics dimension name

4 files changed, 104 insertions, 76 deletions

diff --git a/libnetdata/procfile/procfile.c b/libnetdata/procfile/procfile.c
index b4ca025ec3..eb04316c30 100644
--- a/libnetdata/procfile/procfile.c
+++ b/libnetdata/procfile/procfile.c
@@ -22,16 +22,21 @@ size_t procfile_max_allocation = PROCFILE_INCREMENT_BUFFER;
 // ----------------------------------------------------------------------------
 
 char *procfile_filename(procfile *ff) {
-    if(ff->filename[0]) return ff->filename;
+    if(ff->filename)
+        return ff->filename;
 
+    char filename[FILENAME_MAX + 1];
     char buffer[FILENAME_MAX + 1];
     snprintfz(buffer, FILENAME_MAX, "/proc/self/fd/%d", ff->fd);
 
-    ssize_t l = readlink(buffer, ff->filename, FILENAME_MAX);
+    ssize_t l = readlink(buffer, filename, FILENAME_MAX);
     if(unlikely(l == -1))
-        snprintfz(ff->filename, FILENAME_MAX, "unknown filename for fd %d", ff->fd);
+        snprintfz(filename, FILENAME_MAX, "unknown filename for fd %d", ff->fd);
     else
-        ff->filename[l] = '\0';
+        filename[l] = '\0';
+
+
+    ff->filename = strdupz(filename);
 
     // on non-linux systems, something like this will be needed
     // fcntl(ff->fd, F_GETPATH, ff->filename)
@@ -141,8 +146,9 @@ void procfile_close(procfile *ff) {
 
     debug(D_PROCFILE, PF_PREFIX ": Closing file '%s'", procfile_filename(ff));
 
-    if(likely(ff->lines)) procfile_lines_free(ff->lines);
-    if(likely(ff->words)) procfile_words_free(ff->words);
+    freez(ff->filename);
+    procfile_lines_free(ff->lines);
+    procfile_words_free(ff->words);
 
     if(likely(ff->fd != -1)) close(ff->fd);
     freez(ff);
@@ -319,40 +325,31 @@ procfile *procfile_readall(procfile *ff) {
     return ff;
 }
 
-NOINLINE
-static void procfile_set_separators(procfile *ff, const char *separators) {
-    static PF_CHAR_TYPE def[256];
-    static char initialized = 0;
-
-    if(unlikely(!initialized)) {
-        // this is thread safe
-        // if initialized is zero, multiple threads may be executing
-        // this code at the same time, setting in def[] the exact same values
-        int i = 256;
-        while(i--) {
-            if(unlikely(i == '\n' || i == '\r'))
-                def[i] = PF_CHAR_IS_NEWLINE;
+static PF_CHAR_TYPE procfile_default_separators[256];
+__attribute__((constructor)) void procfile_initialize_default_separators(void) {
+    int i = 256;
+    while(i--) {
+        if(unlikely(i == '\n' || i == '\r'))
+            procfile_default_separators[i] = PF_CHAR_IS_NEWLINE;
 
-            else if(unlikely(isspace(i) || !isprint(i)))
-                def[i] = PF_CHAR_IS_SEPARATOR;
+        else if(unlikely(isspace(i) || !isprint(i)))
+            procfile_default_separators[i] = PF_CHAR_IS_SEPARATOR;
 
-            else
-                def[i] = PF_CHAR_IS_WORD;
-        }
-
-        initialized = 1;
+        else
+            procfile_default_separators[i] = PF_CHAR_IS_WORD;
     }
+}
 
-    // copy the default
-    PF_CHAR_TYPE *ffs = ff->separators, *ffd = def, *ffe = &def[256];
-    while(ffd != ffe)
-        *ffs++ = *ffd++;
-
+NOINLINE
+static void procfile_set_separators(procfile *ff, const char *separators) {
     // set the separators
     if(unlikely(!separators))
         separators = " \t=|";
 
-    ffs = ff->separators;
+    // copy the default
+    memcpy(ff->separators, procfile_default_separators, 256 * sizeof(PF_CHAR_TYPE));
+
+    PF_CHAR_TYPE *ffs = ff->separators;
     const char *s = separators;
     while(*s)
         ffs[(int)*s++] = PF_CHAR_IS_SEPARATOR;
@@ -416,8 +413,7 @@ procfile *procfile_open(const char *filename, const char *separators, uint32_t f
     procfile *ff = mallocz(sizeof(procfile) + size);
 
     //strncpyz(ff->filename, filename, FILENAME_MAX);
-    ff->filename[0] = '\0';
-
+    ff->filename = NULL;
     ff->fd = fd;
     ff->size = size;
     ff->len = 0;
@@ -449,7 +445,8 @@ procfile *procfile_reopen(procfile *ff, const char *filename, const char *separa
     // info("PROCFILE: opened '%s' on fd %d", filename, ff->fd);
 
     //strncpyz(ff->filename, filename, FILENAME_MAX);
-    ff->filename[0] = '\0';
+    freez(ff->filename);
+    ff->filename = NULL;
     ff->flags = flags;
 
     // do not do the separators again if NULL is given
diff --git a/libnetdata/procfile/procfile.h b/libnetdata/procfile/procfile.h
index 5d45e4028c..cae4ad4846 100644
--- a/libnetdata/procfile/procfile.h
+++ b/libnetdata/procfile/procfile.h
@@ -37,7 +37,7 @@ typedef struct {
 #define PROCFILE_FLAG_DEFAULT             0x00000000
 #define PROCFILE_FLAG_NO_ERROR_ON_FILE_IO 0x00000001
 
-typedef enum procfile_separator {
+typedef enum __attribute__ ((__packed__)) procfile_separator {
     PF_CHAR_IS_SEPARATOR,
     PF_CHAR_IS_NEWLINE,
     PF_CHAR_IS_WORD,
@@ -46,17 +46,16 @@ typedef enum procfile_separator {
     PF_CHAR_IS_CLOSE
 } PF_CHAR_TYPE;
 
-typedef struct {
-    char filename[FILENAME_MAX + 1]; // not populated until profile_filename() is called
-
+typedef struct procfile {
+    char *filename;                 // not populated until procfile_filename() is called
     uint32_t flags;
-    int fd;               // the file descriptor
-    size_t len;           // the bytes we have placed into data
-    size_t size;          // the bytes we have allocated for data
+    int fd;                         // the file descriptor
+    size_t len;                     // the bytes we have placed into data
+    size_t size;                    // the bytes we have allocated for data
     pflines *lines;
     pfwords *words;
     PF_CHAR_TYPE separators[256];
-    char data[];          // allocated buffer to keep file contents
+    char data[];                    // allocated buffer to keep file contents
 } procfile;
 
 // close the proc file and free all related memory
diff --git a/libnetdata/worker_utilization/worker_utilization.c b/libnetdata/worker_utilization/worker_utilization.c
index 14b8926e04..afaff209b5 100644
--- a/libnetdata/worker_utilization/worker_utilization.c
+++ b/libnetdata/worker_utilization/worker_utilization.c
@@ -44,35 +44,55 @@ struct worker {
     struct worker *prev;
 };
 
-static netdata_mutex_t workers_base_lock = NETDATA_MUTEX_INITIALIZER;
-static __thread struct worker *worker = NULL;
-static Pvoid_t workers_per_workname_JudyHS_array = NULL;
+struct workers_workname {                           // this is what we add to JudyHS
+    SPINLOCK spinlock;
+    struct worker *base;
+};
+
+static struct workers_globals {
+    SPINLOCK spinlock;
+    Pvoid_t worknames_JudyHS;
+
+} workers_globals = {                               // workers globals, the base of all worknames
+        .spinlock = NETDATA_SPINLOCK_INITIALIZER,   // a lock for the worknames index
+        .worknames_JudyHS = NULL,                   // the worknames index
+};
 
-void worker_register(const char *workname) {
+static __thread struct worker *worker = NULL; // the current thread worker
+
+void worker_register(const char *name) {
     if(unlikely(worker)) return;
 
     worker = callocz(1, sizeof(struct worker));
     worker->pid = gettid();
     worker->tag = strdupz(netdata_thread_tag());
-    worker->workname = strdupz(workname);
+    worker->workname = strdupz(name);
 
     usec_t now = now_monotonic_usec();
     worker->statistics_last_checkpoint = now;
     worker->last_action_timestamp = now;
     worker->last_action = WORKER_IDLE;
 
-    size_t workname_size = strlen(workname) + 1;
-    netdata_mutex_lock(&workers_base_lock);
+    size_t name_size = strlen(name) + 1;
+    netdata_spinlock_lock(&workers_globals.spinlock);
 
-    Pvoid_t *PValue = JudyHSGet(workers_per_workname_JudyHS_array, (void *)workname, workname_size);
+    Pvoid_t *PValue = JudyHSGet(workers_globals.worknames_JudyHS, (void *)name, name_size);
     if(!PValue)
-        PValue = JudyHSIns(&workers_per_workname_JudyHS_array, (void *)workname, workname_size, PJE0);
+        PValue = JudyHSIns(&workers_globals.worknames_JudyHS, (void *)name, name_size, PJE0);
+
+    struct workers_workname *workname = *PValue;
+    if(!workname) {
+        workname = mallocz(sizeof(struct workers_workname));
+        workname->spinlock = NETDATA_SPINLOCK_INITIALIZER;
+        workname->base = NULL;
+        *PValue = workname;
+    }
 
-    struct worker *base = *PValue;
-    DOUBLE_LINKED_LIST_APPEND_UNSAFE(base, worker, prev, next);
-    *PValue = base;
+    netdata_spinlock_lock(&workname->spinlock);
+    DOUBLE_LINKED_LIST_APPEND_UNSAFE(workname->base, worker, prev, next);
+    netdata_spinlock_unlock(&workname->spinlock);
 
-    netdata_mutex_unlock(&workers_base_lock);
+    netdata_spinlock_unlock(&workers_globals.spinlock);
 }
 
 void worker_register_job_custom_metric(size_t job_id, const char *name, const char *units, WORKER_METRIC_TYPE type) {
@@ -105,17 +125,20 @@ void worker_unregister(void) {
     if(unlikely(!worker)) return;
 
     size_t workname_size = strlen(worker->workname) + 1;
-    netdata_mutex_lock(&workers_base_lock);
-    Pvoid_t *PValue = JudyHSGet(workers_per_workname_JudyHS_array, (void *)worker->workname, workname_size);
+    netdata_spinlock_lock(&workers_globals.spinlock);
+    Pvoid_t *PValue = JudyHSGet(workers_globals.worknames_JudyHS, (void *)worker->workname, workname_size);
     if(PValue) {
-        struct worker *base = *PValue;
-        DOUBLE_LINKED_LIST_REMOVE_UNSAFE(base, worker, prev, next);
-        *PValue = base;
-
-        if(!base)
-            JudyHSDel(&workers_per_workname_JudyHS_array, (void *)worker->workname, workname_size, PJE0);
+        struct workers_workname *workname = *PValue;
+        netdata_spinlock_lock(&workname->spinlock);
+        DOUBLE_LINKED_LIST_REMOVE_UNSAFE(workname->base, worker, prev, next);
+        netdata_spinlock_unlock(&workname->spinlock);
+
+        if(!workname->base) {
+            JudyHSDel(&workers_globals.worknames_JudyHS, (void *) worker->workname, workname_size, PJE0);
+            freez(workname);
+        }
     }
-    netdata_mutex_unlock(&workers_base_lock);
+    netdata_spinlock_unlock(&workers_globals.spinlock);
 
     for(int i  = 0; i < WORKER_UTILIZATION_MAX_JOB_TYPES ;i++) {
         string_freez(worker->per_job_type[i].name);
@@ -187,7 +210,7 @@ void worker_set_metric(size_t job_id, NETDATA_DOUBLE value) {
 
 // statistics interface
 
-void workers_foreach(const char *workname, void (*callback)(
+void workers_foreach(const char *name, void (*callback)(
                                                void *data
                                                , pid_t pid
                                                , const char *thread_tag
@@ -203,18 +226,27 @@ void workers_foreach(const char *workname, void (*callback)(
                                                , NETDATA_DOUBLE *job_custom_values
                                                )
                                                , void *data) {
-    netdata_mutex_lock(&workers_base_lock);
+    netdata_spinlock_lock(&workers_globals.spinlock);
     usec_t busy_time, delta;
     size_t i, jobs_started, jobs_running;
 
-    size_t workname_size = strlen(workname) + 1;
-    struct worker *base = NULL;
-    Pvoid_t *PValue = JudyHSGet(workers_per_workname_JudyHS_array, (void *)workname, workname_size);
-    if(PValue)
-        base = *PValue;
+    size_t workname_size = strlen(name) + 1;
+    struct workers_workname *workname;
+    Pvoid_t *PValue = JudyHSGet(workers_globals.worknames_JudyHS, (void *)name, workname_size);
+    if(PValue) {
+        workname = *PValue;
+        netdata_spinlock_lock(&workname->spinlock);
+    }
+    else
+        workname = NULL;
+
+    netdata_spinlock_unlock(&workers_globals.spinlock);
+
+    if(!workname)
+        return;
 
     struct worker *p;
-    DOUBLE_LINKED_LIST_FOREACH_FORWARD(base, p, prev, next) {
+    DOUBLE_LINKED_LIST_FOREACH_FORWARD(workname->base, p, prev, next) {
         usec_t now = now_monotonic_usec();
 
         // find per job type statistics
@@ -326,5 +358,5 @@ void workers_foreach(const char *workname, void (*callback)(
                  );
     }
 
-    netdata_mutex_unlock(&workers_base_lock);
+    netdata_spinlock_unlock(&workname->spinlock);
 }
diff --git a/libnetdata/worker_utilization/worker_utilization.h b/libnetdata/worker_utilization/worker_utilization.h
index 04d24f1f73..f1412e6b49 100644
--- a/libnetdata/worker_utilization/worker_utilization.h
+++ b/libnetdata/worker_utilization/worker_utilization.h
@@ -15,7 +15,7 @@ typedef enum {
     WORKER_METRIC_INCREMENTAL_TOTAL = 4,
 } WORKER_METRIC_TYPE;
 
-void worker_register(const char *workname);
+void worker_register(const char *name);
 void worker_register_job_name(size_t job_id, const char *name);
 void worker_register_job_custom_metric(size_t job_id, const char *name, const char *units, WORKER_METRIC_TYPE type);
 void worker_unregister(void);
@@ -26,7 +26,7 @@ void worker_set_metric(size_t job_id, NETDATA_DOUBLE value);
 
 // statistics interface
 
-void workers_foreach(const char *workname, void (*callback)(
+void workers_foreach(const char *name, void (*callback)(
                                                       void *data
                                                       , pid_t pid
                                                       , const char *thread_tag