From b1b35a56e74de8f63e0f20dd6b629aabf6453994 Mon Sep 17 00:00:00 2001 From: nick black Date: Thu, 15 Jul 2021 20:29:42 -0400 Subject: [PATCH] move stats lock into object with stats struct #1914 --- include/notcurses/notcurses.h | 4 -- src/lib/internal.h | 13 +++- src/lib/notcurses.c | 16 ++--- src/lib/render.c | 46 +++++++------- src/lib/stats.c | 109 +++++++++++++++------------------- 5 files changed, 91 insertions(+), 97 deletions(-) diff --git a/include/notcurses/notcurses.h b/include/notcurses/notcurses.h index dca34253f..c0e1cfbab 100644 --- a/include/notcurses/notcurses.h +++ b/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))); diff --git a/src/lib/internal.h b/src/lib/internal.h index 6a9205c97..75b5c930c 100644 --- a/src/lib/internal.h +++ b/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 diff --git a/src/lib/notcurses.c b/src/lib/notcurses.c index 7707028b7..a96507730 100644 --- a/src/lib/notcurses.c +++ b/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; diff --git a/src/lib/render.c b/src/lib/render.c index 9290742ba..3914fc430 100644 --- a/src/lib/render.c +++ b/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; diff --git a/src/lib/stats.c b/src/lib/stats.c index 719eb634f..4ba521896 100644 --- a/src/lib/stats.c +++ b/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); }