From e8b5cb6c230a183524e1e1c494b1fd9b98f3e72f Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Tue, 22 Mar 2016 12:26:45 +0000 Subject: [PATCH] add VIPS_COUNT_PIXELS, overcomputation tracking added VIPS_COUNT_PIXELS(), use like this: static int vips_shrinkh_gen( VipsRegion *or, ... ) { VIPS_COUNT_PIXELS( or, "vips_shrinkh_gen" ); } and on image close, if more than 100% of the pixels have been calculated, you get a warning only if you enable debugging, since this hurts perf slightly --- ChangeLog | 1 + TODO | 21 +++++++++++++++++---- libvips/include/vips/internal.h | 13 +++++++++++++ libvips/include/vips/region.h | 13 +++++++++++++ libvips/iofuncs/image.c | 21 +++++++++++++++++++++ libvips/iofuncs/init.c | 11 +++++++++++ libvips/iofuncs/region.c | 18 ++++++++++++++++++ libvips/resample/affine.c | 2 ++ libvips/resample/reduceh.cpp | 2 ++ libvips/resample/reducev.cpp | 4 ++++ libvips/resample/shrinkh.c | 2 ++ libvips/resample/shrinkv.c | 2 ++ 12 files changed, 106 insertions(+), 4 deletions(-) diff --git a/ChangeLog b/ChangeLog index 1c9a2ff9..4e6dc7bf 100644 --- a/ChangeLog +++ b/ChangeLog @@ -19,6 +19,7 @@ - python .bandrank() now works like .bandjoin() - vipsthumbnail --interpolator and --sharpen switches are deprecated - switches to disable PPM, Rad and Analyze support +- added VIPS_COUNT_PIXELS(), overcomputation tracking 27/1/16 started 8.2.3 - fix a crash with SPARC byte-order labq vips images diff --git a/TODO b/TODO index 1ca04cdf..1a6ddf76 100644 --- a/TODO +++ b/TODO @@ -1,7 +1,3 @@ -- could instrument vips_region_prepare() and track number of pixels calculated - for each image ... on image close (or evelend?) print %computed, values over - 100% would indicate overcomputation - - if we do vips_region_prepare( VipsRect { 0, 0, 100, 10 } ) @@ -10,6 +6,23 @@ do we recalculate the whole region, or shift the calculated bits down and just fetch another line? + how would we calculate a small part of a region? + + vips_region_fill() would need to do this + + instrument a bit and see what kinds of buffers we are seeing + + if we see + + buffer.done: true + buffer.area: {0, 0, 100, 10 } + + and we are asked for + + buffer.area: {0, 1, 100, 10 } + + we can scroll, then use vips_region_prepare_to() to calculate just the + new pixels ... would this fix it? diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index 9ce71efc..ca92fa89 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -233,6 +233,19 @@ char *vips__make_xml_metadata( const char *domain, VipsImage *image ); void vips__cairo2rgba( guint32 *buf, int n ); +#ifdef DEBUG_LEAK +extern GQuark vips__image_pixels_quark; +#endif /*DEBUG_LEAK*/ + +/* With DEBUG_LEAK, hang one of these off each image and count pixels + * calculated. + */ +typedef struct _VipsImagePixels { + const char *nickname; + gint64 tpels; /* Number of pels we expect to calculate */ + gint64 npels; /* Number of pels calculated so far */ +} VipsImagePixels; + #ifdef __cplusplus } #endif /*__cplusplus*/ diff --git a/libvips/include/vips/region.h b/libvips/include/vips/region.h index cd5f9e7a..5c9439d4 100644 --- a/libvips/include/vips/region.h +++ b/libvips/include/vips/region.h @@ -125,6 +125,19 @@ void vips_region_invalidate( VipsRegion *reg ); void vips_region_dump_all( void ); +#ifdef DEBUG_LEAK +void vips__region_count_pixels( VipsRegion *region, const char *nickname ); +#endif /*DEBUG_LEAK*/ + +/* Use this to count pixels passing through key points. Handy for spotting bad + * overcomputation. + */ +#ifdef DEBUG_LEAK +#define VIPS_COUNT_PIXELS( R, N ) vips__region_count_pixels( R, N ) +#else /*!DEBUG_LEAK*/ +#define VIPS_COUNT_PIXELS( R, N ) +#endif /*DEBUG_LEAK*/ + /* Macros on VipsRegion. * VIPS_REGION_LSKIP() add to move down line * VIPS_REGION_N_ELEMENTS() number of elements across region diff --git a/libvips/iofuncs/image.c b/libvips/iofuncs/image.c index 3c739e35..4a3bd3d2 100644 --- a/libvips/iofuncs/image.c +++ b/libvips/iofuncs/image.c @@ -480,6 +480,22 @@ vips_image_dispose( GObject *gobject ) VIPS_DEBUG_MSG( "vips_image_dispose: %p\n", gobject ); +#ifdef DEBUG_LEAK +{ + VipsImagePixels *pixels = g_object_get_qdata( G_OBJECT( image ), + vips__image_pixels_quark ); + + if( pixels && + pixels->tpels ) { + int compute_percent = 100.0 * pixels->npels / pixels->tpels; + + if( compute_percent > 100 ) + printf( "vips_image_dispose: %s computed %d%%\n", + pixels->nickname, compute_percent ); + } +} +#endif /*DEBUG_LEAK*/ + vips_object_preclose( VIPS_OBJECT( gobject ) ); /* We have to junk the fd in dispose, since we run this for rewind and @@ -1368,6 +1384,11 @@ vips_image_init( VipsImage *image ) image->sizeof_header = VIPS_SIZEOF_HEADER; image->mode = g_strdup( "p" ); + +#ifdef DEBUG_LEAK + g_object_set_qdata_full( G_OBJECT( image ), vips__image_pixels_quark, + g_new0( VipsImagePixels, 1 ), (GDestroyNotify) g_free ); +#endif /*DEBUG_LEAK*/ } int diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c index c7780bf8..dc314813 100644 --- a/libvips/iofuncs/init.c +++ b/libvips/iofuncs/init.c @@ -102,6 +102,12 @@ static char *vips__argv0 = NULL; */ int vips__leak = 0; +#ifdef DEBUG_LEAK +/* Count pixels processed per image here. + */ +GQuark vips__image_pixels_quark = 0; +#endif /*DEBUG_LEAK*/ + /** * vips_get_argv0: * @@ -394,6 +400,11 @@ vips_init( const char *argv0 ) atexit( vips_shutdown ); #endif /*HAVE_ATEXIT*/ +#ifdef DEBUG_LEAK + vips__image_pixels_quark = + g_quark_from_static_string( "vips-image-pixels" ); +#endif /*DEBUG_LEAK*/ + done = TRUE; vips__thread_gate_stop( "init: startup" ); diff --git a/libvips/iofuncs/region.c b/libvips/iofuncs/region.c index c1514831..ed338219 100644 --- a/libvips/iofuncs/region.c +++ b/libvips/iofuncs/region.c @@ -1631,3 +1631,21 @@ vips_region_dump_all( void ) g_mutex_unlock( vips__global_lock ); } #endif /*VIPS_DEBUG*/ + +#ifdef DEBUG_LEAK +void +vips__region_count_pixels( VipsRegion *region, const char *nickname ) +{ + VipsImage *image = region->im; + VipsImagePixels *pixels = g_object_get_qdata( G_OBJECT( image ), + vips__image_pixels_quark ); + + g_mutex_lock( vips__global_lock ); + if( !pixels->tpels ) + pixels->tpels = VIPS_IMAGE_N_PELS( image ); + if( !pixels->nickname ) + pixels->nickname = nickname; + pixels->npels += region->valid.width * region->valid.height; + g_mutex_unlock( vips__global_lock ); +} +#endif /*DEBUG_LEAK*/ diff --git a/libvips/resample/affine.c b/libvips/resample/affine.c index fc34cc3a..de9ded32 100644 --- a/libvips/resample/affine.c +++ b/libvips/resample/affine.c @@ -367,6 +367,8 @@ vips_affine_gen( VipsRegion *or, void *seq, void *a, void *b, gboolean *stop ) VIPS_GATE_STOP( "vips_affine_gen: work" ); + VIPS_COUNT_PIXELS( or, "vips_affine_gen" ); + return( 0 ); } diff --git a/libvips/resample/reduceh.cpp b/libvips/resample/reduceh.cpp index 79149585..1b9235b3 100644 --- a/libvips/resample/reduceh.cpp +++ b/libvips/resample/reduceh.cpp @@ -399,6 +399,8 @@ vips_reduceh_gen( VipsRegion *out_region, void *seq, VIPS_GATE_STOP( "vips_reduceh_gen: work" ); + VIPS_COUNT_PIXELS( out_region, "vips_reduceh_gen" ); + return( 0 ); } diff --git a/libvips/resample/reducev.cpp b/libvips/resample/reducev.cpp index 170cd92d..b2447c65 100644 --- a/libvips/resample/reducev.cpp +++ b/libvips/resample/reducev.cpp @@ -591,6 +591,8 @@ vips_reducev_gen( VipsRegion *out_region, void *vseq, VIPS_GATE_STOP( "vips_reducev_gen: work" ); + VIPS_COUNT_PIXELS( out_region, "vips_reducev_gen" ); + return( 0 ); } @@ -666,6 +668,8 @@ vips_reducev_vector_gen( VipsRegion *out_region, void *vseq, VIPS_GATE_STOP( "vips_reducev_vector_gen: work" ); + VIPS_COUNT_PIXELS( out_region, "vips_reducev_vector_gen" ); + return( 0 ); } diff --git a/libvips/resample/shrinkh.c b/libvips/resample/shrinkh.c index e64c257f..68f1583f 100644 --- a/libvips/resample/shrinkh.c +++ b/libvips/resample/shrinkh.c @@ -218,6 +218,8 @@ vips_shrinkh_gen( VipsRegion *or, void *seq, VIPS_GATE_STOP( "vips_shrinkh_gen: work" ); } + VIPS_COUNT_PIXELS( or, "vips_shrinkh_gen" ); + return( 0 ); } diff --git a/libvips/resample/shrinkv.c b/libvips/resample/shrinkv.c index fc781efd..0f6c1e37 100644 --- a/libvips/resample/shrinkv.c +++ b/libvips/resample/shrinkv.c @@ -316,6 +316,8 @@ vips_shrinkv_gen( VipsRegion *or, void *vseq, VIPS_GATE_STOP( "vips_shrinkv_gen: work" ); } + VIPS_COUNT_PIXELS( or, "vips_shrinkv_gen" ); + return( 0 ); }