move stats lock into object with stats struct #1914

include/notcurses/notcurses.h

@@ -1446,7 +1446,6 @@ typedef struct ncstats {
   uint64_t sprixelelisions;  // sprixel elision count
   uint64_t sprixelbytes;     // sprixel bytes emitted
   uint64_t input_errors;     // errors processing control sequences/utf8
-  pthread_mutex_t lock;      // FIXME ensure alignment on cacheline
 } ncstats;
 
 // Allocate an ncstats object. Use this rather than allocating your own, since
@@ -1455,9 +1454,6 @@ API ALLOC ncstats* notcurses_stats_alloc(const struct notcurses* nc
                                          __attribute__ ((unused)))
   __attribute__ ((nonnull (1)));
 
-// Destroy an ncstats object, and any associated resources.
-API void notcurses_stats_destroy(ncstats* stats);
-
 // Acquire an atomic snapshot of the Notcurses object's stats.
 API void notcurses_stats(struct notcurses* nc, ncstats* stats)
   __attribute__ ((nonnull (1, 2)));

src/lib/internal.h

@@ -325,6 +325,15 @@ typedef struct ncpile {
   sprixel* sprixelcache;      // list of sprixels
 } ncpile;
 
+// various moving parts within a notcurses context (and the user) might need to
+// access the stats object, so throw a lock on it. we don't want the lock in
+// the actual structure since (a) it's usually unnecessary and (b) it breaks
+// memset() and memcpy().
+typedef struct ncsharedstats {
+  pthread_mutex_t lock;
+  ncstats s;
+} ncsharedstats;
+
 // the standard pile can be reached through ->stdplane.
 typedef struct notcurses {
   ncplane* stdplane; // standard plane, covers screen
@@ -349,8 +358,8 @@ typedef struct notcurses {
   int cursory;    // desired cursor placement according to user.
   int cursorx;    // -1 is don't-care, otherwise moved here after each render.
 
-  ncstats stats;  // some statistics across the lifetime of the notcurses ctx
-  ncstats stashed_stats; // retain across a notcurses_stats_reset(), to print in closing banner
+  ncsharedstats stats;   // some statistics across the lifetime of the context
+  ncstats stashed_stats; // retain across a context reset, for closing banner
 
   FILE* ttyfp;    // FILE* for writing rasterized data
   int ttyfd;      // file descriptor for controlling tty

src/lib/notcurses.c

@@ -317,8 +317,8 @@ void free_plane(ncplane* p){
     if(ncplane_pile(p)){
       notcurses* nc = ncplane_notcurses(p);
       pthread_mutex_lock(&nc->stats.lock);
-        --ncplane_notcurses(p)->stats.planes;
-        ncplane_notcurses(p)->stats.fbbytes -= sizeof(*p->fb) * p->leny * p->lenx;
+        --ncplane_notcurses(p)->stats.s.planes;
+        ncplane_notcurses(p)->stats.s.fbbytes -= sizeof(*p->fb) * p->leny * p->lenx;
       pthread_mutex_unlock(&nc->stats.lock);
       if(p->above == NULL && p->below == NULL){
         pthread_mutex_lock(&nc->pilelock);
@@ -505,8 +505,8 @@ ncplane* ncplane_new_internal(notcurses* nc, ncplane* n,
         make_ncpile(nc, p);
       }
       pthread_mutex_lock(&nc->stats.lock);
-        nc->stats.fbbytes += fbsize;
-        ++nc->stats.planes;
+        nc->stats.s.fbbytes += fbsize;
+        ++nc->stats.s.planes;
       pthread_mutex_unlock(&nc->stats.lock);
     pthread_mutex_unlock(&nc->pilelock);
   }
@@ -739,8 +739,8 @@ int ncplane_resize_internal(ncplane* n, int keepy, int keepx, int keepleny,
   }
   nccell* preserved = n->fb;
   pthread_mutex_lock(&nc->stats.lock);
-  ncplane_notcurses(n)->stats.fbbytes -= sizeof(*preserved) * (rows * cols);
-  ncplane_notcurses(n)->stats.fbbytes += fbsize;
+    ncplane_notcurses(n)->stats.s.fbbytes -= sizeof(*preserved) * (rows * cols);
+    ncplane_notcurses(n)->stats.s.fbbytes += fbsize;
   pthread_mutex_unlock(&nc->stats.lock);
   n->fb = fb;
   const int oldabsy = n->absy;
@@ -914,7 +914,7 @@ init_banner(const notcurses* nc){
     }else{
       fprintf(nc->ttyfp, "%d rows %d cols (%sB) %zuB crend %d colors",
               nc->stdplane->leny, nc->stdplane->lenx,
-              bprefix(nc->stats.fbbytes, 1, prefixbuf, 0),
+              bprefix(nc->stats.s.fbbytes, 1, prefixbuf, 0),
               sizeof(struct crender), nc->tcache.caps.colors);
     }
     const char* setaf;
@@ -1067,7 +1067,7 @@ notcurses* notcurses_core_init(const notcurses_options* opts, FILE* outfp){
   ret->cursory = ret->cursorx = -1;
   memset(&ret->stats, 0, sizeof(ret->stats));
   memset(&ret->stashed_stats, 0, sizeof(ret->stashed_stats));
-  reset_stats(&ret->stats);
+  reset_stats(&ret->stats.s);
   reset_stats(&ret->stashed_stats);
   ret->ttyfp = outfp;
   ret->renderfp = opts->renderfp;

src/lib/render.c

@@ -730,7 +730,7 @@ raster_defaults(notcurses* nc, bool fgdef, bool bgdef, FILE* out){
   bool mustsetfg = fgdef && !nc->rstate.fgdefelidable;
   bool mustsetbg = bgdef && !nc->rstate.bgdefelidable;
   if(!mustsetfg && !mustsetbg){ // needn't emit anything
-    ++nc->stats.defaultelisions;
+    ++nc->stats.s.defaultelisions;
     return 0;
   }else if((mustsetfg && mustsetbg) || !fgop || !bgop){
     if(term_emit(op, out, false)){
@@ -757,7 +757,7 @@ raster_defaults(notcurses* nc, bool fgdef, bool bgdef, FILE* out){
     nc->rstate.bgelidable = false;
     nc->rstate.bgpalelidable = false;
   }
-  ++nc->stats.defaultemissions;
+  ++nc->stats.s.defaultemissions;
   return 0;
 }
 
@@ -767,12 +767,12 @@ emit_fg_palindex(notcurses* nc, FILE* out, const nccell* srccell){
   unsigned palfg = nccell_fg_palindex(srccell);
   // we overload lastr for the palette index; both are 8 bits
   if(nc->rstate.fgpalelidable && nc->rstate.lastr == palfg){
-    ++nc->stats.fgelisions;
+    ++nc->stats.s.fgelisions;
   }else{
     if(term_fg_palindex(nc, out, palfg)){
       return -1;
     }
-    ++nc->stats.fgemissions;
+    ++nc->stats.s.fgemissions;
     nc->rstate.fgpalelidable = true;
   }
   nc->rstate.lastr = palfg;
@@ -785,12 +785,12 @@ static int
 emit_bg_palindex(notcurses* nc, FILE* out, const nccell* srccell){
   unsigned palbg = nccell_bg_palindex(srccell);
   if(nc->rstate.bgpalelidable && nc->rstate.lastbr == palbg){
-    ++nc->stats.bgelisions;
+    ++nc->stats.s.bgelisions;
   }else{
     if(term_bg_palindex(nc, out, palbg)){
       return -1;
     }
-    ++nc->stats.bgemissions;
+    ++nc->stats.s.bgemissions;
     nc->rstate.bgpalelidable = true;
   }
   nc->rstate.lastr = palbg;
@@ -860,9 +860,9 @@ clean_sprixels(notcurses* nc, ncpile* p, FILE* out){
       // FIXME might not need this if it was only an upload
       nc->rstate.hardcursorpos = true;
       parent = &s->next;
-      ++nc->stats.sprixelemissions;
+      ++nc->stats.s.sprixelemissions;
     }else{
-      ++nc->stats.sprixelelisions;
+      ++nc->stats.s.sprixelelisions;
       parent = &s->next;
     }
 //fprintf(stderr, "SPRIXEL STATE: %d\n", s->invalidated);
@@ -974,7 +974,7 @@ rasterize_core(notcurses* nc, const ncpile* p, FILE* out, unsigned phase){
       if(!rvec[damageidx].s.damaged){
         // no need to emit a cell; what we rendered appears to already be
         // here. no updates are performed to elision state nor lastframe.
-        ++nc->stats.cellelisions;
+        ++nc->stats.s.cellelisions;
         if(nccell_wide_left_p(srccell)){
           ++x;
         }
@@ -983,7 +983,7 @@ rasterize_core(notcurses* nc, const ncpile* p, FILE* out, unsigned phase){
         // in the first text phase, we draw only those glyphs where the glyph
         // was not above a sprixel (and the cell is damaged). in the second
         // phase, we draw everything that remains damaged.
-        ++nc->stats.cellemissions;
+        ++nc->stats.s.cellemissions;
         if(goto_location(nc, out, y, x)){
           return -1;
         }
@@ -1015,12 +1015,12 @@ rasterize_core(notcurses* nc, const ncpile* p, FILE* out, unsigned phase){
         }else if(!nccell_fg_default_p(srccell)){ // rgb foreground
           nccell_fg_rgb8(srccell, &r, &g, &b);
           if(nc->rstate.fgelidable && nc->rstate.lastr == r && nc->rstate.lastg == g && nc->rstate.lastb == b){
-            ++nc->stats.fgelisions;
+            ++nc->stats.s.fgelisions;
           }else{
             if(term_fg_rgb8(&nc->tcache, out, r, g, b)){
               return -1;
             }
-            ++nc->stats.fgemissions;
+            ++nc->stats.s.fgemissions;
             nc->rstate.fgelidable = true;
           }
           nc->rstate.lastr = r; nc->rstate.lastg = g; nc->rstate.lastb = b;
@@ -1032,7 +1032,7 @@ rasterize_core(notcurses* nc, const ncpile* p, FILE* out, unsigned phase){
         //  * we do not use the background, because the cell is all-foreground,
         //  * the previous was non-default, and matches what we have now, or
         if(nobackground){
-          ++nc->stats.bgelisions;
+          ++nc->stats.s.bgelisions;
         }else if(nccell_bg_palindex_p(srccell)){ // palette-indexed background
           if(emit_bg_palindex(nc, out, srccell)){
             return -1;
@@ -1040,12 +1040,12 @@ rasterize_core(notcurses* nc, const ncpile* p, FILE* out, unsigned phase){
         }else if(!nccell_bg_default_p(srccell)){ // rgb background
           nccell_bg_rgb8(srccell, &br, &bg, &bb);
           if(nc->rstate.bgelidable && nc->rstate.lastbr == br && nc->rstate.lastbg == bg && nc->rstate.lastbb == bb){
-            ++nc->stats.bgelisions;
+            ++nc->stats.s.bgelisions;
           }else{
             if(term_bg_rgb8(&nc->tcache, out, br, bg, bb)){
               return -1;
             }
-            ++nc->stats.bgemissions;
+            ++nc->stats.s.bgemissions;
             nc->rstate.bgelidable = true;
           }
           nc->rstate.lastbr = br; nc->rstate.lastbg = bg; nc->rstate.lastbb = bb;
@@ -1110,7 +1110,7 @@ notcurses_rasterize_inner(notcurses* nc, ncpile* p, FILE* out, unsigned* asu){
   }
   sprixelbytes += rasprixelbytes;
   pthread_mutex_lock(&nc->stats.lock);
-    nc->stats.sprixelbytes += sprixelbytes;
+    nc->stats.s.sprixelbytes += sprixelbytes;
   pthread_mutex_unlock(&nc->stats.lock);
   logdebug("Glyph phase 2\n");
   if(rasterize_core(nc, p, out, 1)){
@@ -1162,7 +1162,7 @@ raster_and_write(notcurses* nc, ncpile* p, FILE* out){
   size_t moffset = 0;
   if(basu){
     if(useasu){
-      ++nc->stats.appsync_updates;
+      ++nc->stats.s.appsync_updates;
     }else{
       moffset = strlen(basu);
     }
@@ -1272,7 +1272,7 @@ int notcurses_refresh(notcurses* nc, int* restrict dimy, int* restrict dimx){
   if(ret < 0){
     return -1;
   }
-  ++nc->stats.refreshes;
+  ++nc->stats.s.refreshes;
   return 0;
 }
 
@@ -1360,9 +1360,9 @@ int ncpile_rasterize(ncplane* n){
   // accepts -1 as an indication of failure
   clock_gettime(CLOCK_MONOTONIC, &writedone);
   pthread_mutex_lock(&nc->stats.lock);
-    update_render_bytes(&nc->stats, bytes);
-    update_raster_stats(&rasterdone, &start, &nc->stats);
-    update_write_stats(&writedone, &rasterdone, &nc->stats, bytes);
+    update_render_bytes(&nc->stats.s, bytes);
+    update_raster_stats(&rasterdone, &start, &nc->stats.s);
+    update_write_stats(&writedone, &rasterdone, &nc->stats.s, bytes);
   pthread_mutex_unlock(&nc->stats.lock);
   if(bytes < 0){
     return -1;
@@ -1404,7 +1404,7 @@ int ncpile_render(ncplane* n){
   ncpile_render_internal(n, pile->crender, pile->dimy, pile->dimx);
   clock_gettime(CLOCK_MONOTONIC, &renderdone);
   pthread_mutex_lock(&nc->stats.lock);
-  update_render_stats(&renderdone, &start, &nc->stats);
+    update_render_stats(&renderdone, &start, &nc->stats.s);
   pthread_mutex_unlock(&nc->stats.lock);
   return 0;
 }
@@ -1433,7 +1433,7 @@ int ncpile_render_to_buffer(ncplane* p, char** buf, size_t* buflen){
   fseeko(nc->rstate.mstreamfp, 0, SEEK_SET);
   int bytes = notcurses_rasterize_inner(nc, ncplane_pile(p), nc->rstate.mstreamfp, &useasu);
   pthread_mutex_lock(&nc->stats.lock);
-    update_render_bytes(&nc->stats, bytes);
+    update_render_bytes(&nc->stats.s, bytes);
   pthread_mutex_unlock(&nc->stats.lock);
   if(bytes < 0){
     return -1;

src/lib/stats.c

@@ -94,20 +94,9 @@ ncstats* notcurses_stats_alloc(const notcurses* nc __attribute__ ((unused))){
   if(ret == NULL){
     return NULL;
   }
-  if(pthread_mutex_init(&ret->lock, NULL)){
-    free(ret);
-    return NULL;
-  }
   return ret;
 }
 
-void notcurses_stats_destroy(ncstats* stats){
-  if(stats){
-    pthread_mutex_destroy(&stats->lock);
-    free(stats);
-  }
-}
-
 void notcurses_stats_reset(notcurses* nc, ncstats* stats){
   pthread_mutex_lock(&nc->stats.lock);
     if(stats){
@@ -116,56 +105,56 @@ void notcurses_stats_reset(notcurses* nc, ncstats* stats){
     // add the stats to the stashed stats, so that we can show true totals on
     // shutdown in the closing banner
     ncstats* stash = &nc->stashed_stats;
-    if(nc->stats.render_min_ns < stash->render_min_ns){
-      stash->render_min_ns = nc->stats.render_min_ns;
-    }
-    if(nc->stats.render_min_bytes < stash->render_min_bytes){
-      stash->render_min_bytes = nc->stats.render_min_bytes;
-    }
-    if(nc->stats.raster_min_ns < stash->raster_min_ns){
-      stash->raster_min_ns = nc->stats.raster_min_ns;
-    }
-    if(nc->stats.writeout_min_ns < stash->writeout_min_ns){
-      stash->writeout_min_ns = nc->stats.writeout_min_ns;
-    }
-    if(nc->stats.render_max_ns > stash->render_max_ns){
-      stash->render_max_ns = nc->stats.render_max_ns;
-    }
-    if(nc->stats.render_max_bytes > stash->render_max_bytes){
-      stash->render_max_bytes = nc->stats.render_max_bytes;
-    }
-    if(nc->stats.raster_max_ns > stash->raster_max_ns){
-      stash->raster_max_ns = nc->stats.raster_max_ns;
-    }
-    if(nc->stats.writeout_max_ns > stash->writeout_max_ns){
-      stash->writeout_max_ns = nc->stats.writeout_max_ns;
-    }
-    stash->writeout_ns += nc->stats.writeout_ns;
-    stash->raster_ns += nc->stats.raster_ns;
-    stash->render_ns += nc->stats.render_ns;
-    stash->render_bytes += nc->stats.render_bytes;
-    stash->failed_renders += nc->stats.failed_renders;
-    stash->failed_writeouts += nc->stats.failed_writeouts;
-    stash->renders += nc->stats.renders;
-    stash->writeouts += nc->stats.writeouts;
-    stash->cellelisions += nc->stats.cellelisions;
-    stash->cellemissions += nc->stats.cellemissions;
-    stash->fgelisions += nc->stats.fgelisions;
-    stash->fgemissions += nc->stats.fgemissions;
-    stash->bgelisions += nc->stats.bgelisions;
-    stash->bgemissions += nc->stats.bgemissions;
-    stash->defaultelisions += nc->stats.defaultelisions;
-    stash->defaultemissions += nc->stats.defaultemissions;
-    stash->refreshes += nc->stats.refreshes;
-    stash->sprixelemissions += nc->stats.sprixelemissions;
-    stash->sprixelelisions += nc->stats.sprixelelisions;
-    stash->sprixelbytes += nc->stats.sprixelbytes;
-    stash->appsync_updates += nc->stats.appsync_updates;
-    stash->input_errors += nc->stats.input_errors;
+    if(nc->stats.s.render_min_ns < stash->render_min_ns){
+      stash->render_min_ns = nc->stats.s.render_min_ns;
+    }
+    if(nc->stats.s.render_min_bytes < stash->render_min_bytes){
+      stash->render_min_bytes = nc->stats.s.render_min_bytes;
+    }
+    if(nc->stats.s.raster_min_ns < stash->raster_min_ns){
+      stash->raster_min_ns = nc->stats.s.raster_min_ns;
+    }
+    if(nc->stats.s.writeout_min_ns < stash->writeout_min_ns){
+      stash->writeout_min_ns = nc->stats.s.writeout_min_ns;
+    }
+    if(nc->stats.s.render_max_ns > stash->render_max_ns){
+      stash->render_max_ns = nc->stats.s.render_max_ns;
+    }
+    if(nc->stats.s.render_max_bytes > stash->render_max_bytes){
+      stash->render_max_bytes = nc->stats.s.render_max_bytes;
+    }
+    if(nc->stats.s.raster_max_ns > stash->raster_max_ns){
+      stash->raster_max_ns = nc->stats.s.raster_max_ns;
+    }
+    if(nc->stats.s.writeout_max_ns > stash->writeout_max_ns){
+      stash->writeout_max_ns = nc->stats.s.writeout_max_ns;
+    }
+    stash->writeout_ns += nc->stats.s.writeout_ns;
+    stash->raster_ns += nc->stats.s.raster_ns;
+    stash->render_ns += nc->stats.s.render_ns;
+    stash->render_bytes += nc->stats.s.render_bytes;
+    stash->failed_renders += nc->stats.s.failed_renders;
+    stash->failed_writeouts += nc->stats.s.failed_writeouts;
+    stash->renders += nc->stats.s.renders;
+    stash->writeouts += nc->stats.s.writeouts;
+    stash->cellelisions += nc->stats.s.cellelisions;
+    stash->cellemissions += nc->stats.s.cellemissions;
+    stash->fgelisions += nc->stats.s.fgelisions;
+    stash->fgemissions += nc->stats.s.fgemissions;
+    stash->bgelisions += nc->stats.s.bgelisions;
+    stash->bgemissions += nc->stats.s.bgemissions;
+    stash->defaultelisions += nc->stats.s.defaultelisions;
+    stash->defaultemissions += nc->stats.s.defaultemissions;
+    stash->refreshes += nc->stats.s.refreshes;
+    stash->sprixelemissions += nc->stats.s.sprixelemissions;
+    stash->sprixelelisions += nc->stats.s.sprixelelisions;
+    stash->sprixelbytes += nc->stats.s.sprixelbytes;
+    stash->appsync_updates += nc->stats.s.appsync_updates;
+    stash->input_errors += nc->stats.s.input_errors;
 
-    stash->fbbytes = nc->stats.fbbytes;
-    stash->planes = nc->stats.planes;
-    reset_stats(&nc->stats);
+    stash->fbbytes = nc->stats.s.fbbytes;
+    stash->planes = nc->stats.s.planes;
+    reset_stats(&nc->stats.s);
   pthread_mutex_unlock(&nc->stats.lock);
 }