From 4b2c8587b2b88d0d64150ebfbe69f01b5edcc57b Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Mon, 19 Sep 2011 16:44:51 +0100 Subject: [PATCH] vips_malloc() size tracking vips_malloc() now tracks allocation size and can report total mem use. It seems to trigger quite a few nip2 bugs though, I guess we are g_free()ing the result in places (or vice versa). ======= Backtrace: ========= /lib/x86_64-linux-gnu/libc.so.6(+0x78a8f)[0x2aae8e011a8f] /lib/x86_64-linux-gnu/libc.so.6(cfree+0x73)[0x2aae8e0158e3] /home/john/vips/lib/libvips.so.15(vips_free+0xc2)[0x2aae88f7717e] /home/john/GIT/nip2/src/nip2[0x4c9ce2] /home/john/GIT/nip2/src/nip2(path_map_exact+0x63)[0x4ca127] /home/john/GIT/nip2/src/nip2[0x4b5381] --- ChangeLog | 1 + TODO | 5 + libvips/include/vips/memory.h | 4 + libvips/include/vips/object.h | 2 - libvips/include/vips/operation.h | 2 + libvips/iofuncs/init.c | 6 +- libvips/iofuncs/memory.c | 155 +++++++++++++++++++++---------- libvips/iofuncs/operation.c | 2 +- 8 files changed, 125 insertions(+), 52 deletions(-) diff --git a/ChangeLog b/ChangeLog index 033e8824..4f191889 100644 --- a/ChangeLog +++ b/ChangeLog @@ -12,6 +12,7 @@ - fix up VipsPool - add the operation cache - fallback vips_init() +- vips_malloc() tracks allocation size and can report total mem usage 10/8/11 started 7.26.3 - don't use G_VALUE_COLLECT_INIT(), many platforms do not have a glib this diff --git a/TODO b/TODO index 425430a1..35e989df 100644 --- a/TODO +++ b/TODO @@ -1,3 +1,8 @@ +- vips_malloc() now tracks alloc size, but we seem to g_free() the results + sometimes, look into this + + + - cache work ... see notes in file diff --git a/libvips/include/vips/memory.h b/libvips/include/vips/memory.h index 8f7d1903..7af406cd 100644 --- a/libvips/include/vips/memory.h +++ b/libvips/include/vips/memory.h @@ -76,6 +76,10 @@ int vips_free( void *s ); char *vips_strdup( VipsImage *image, const char *str ); +size_t vips_alloc_get_mem( void ); +size_t vips_alloc_get_mem_highwater( void ); +unsigned int vips_alloc_get_allocs( void ); + #ifdef __cplusplus } #endif /*__cplusplus*/ diff --git a/libvips/include/vips/object.h b/libvips/include/vips/object.h index 200d6aa9..89e6b746 100644 --- a/libvips/include/vips/object.h +++ b/libvips/include/vips/object.h @@ -368,8 +368,6 @@ void vips_object_sanity_all( void ); void vips_object_rewind( VipsObject *object ); -int vips_object_build_cache( VipsObject **object ); - #ifdef __cplusplus } #endif /*__cplusplus*/ diff --git a/libvips/include/vips/operation.h b/libvips/include/vips/operation.h index 526a3266..d14006cd 100644 --- a/libvips/include/vips/operation.h +++ b/libvips/include/vips/operation.h @@ -81,6 +81,8 @@ int vips_call_split( const char *operation_name, va_list optional, ... ); void vips_call_options( GOptionGroup *group, VipsOperation *operation ); int vips_call_argv( VipsOperation *operation, int argc, char **argv ); +int vips_operation_build_cache( VipsOperation **operation ); + #ifdef __cplusplus } #endif /*__cplusplus*/ diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c index 94f058e5..314a040f 100644 --- a/libvips/iofuncs/init.c +++ b/libvips/iofuncs/init.c @@ -183,12 +183,12 @@ vips_init( const char *argv0 ) return( 0 ); started = TRUE; - VIPS_SETSTR( vips__argv0, argv0 ); - /* Need gobject etc. */ g_type_init(); + /* Older glibs need this. + */ #ifdef G_THREADS_ENABLED if( !g_thread_supported() ) g_thread_init( NULL ); @@ -197,6 +197,8 @@ vips_init( const char *argv0 ) if( !vips__global_lock ) vips__global_lock = g_mutex_new(); + VIPS_SETSTR( vips__argv0, argv0 ); + prgname = g_path_get_basename( argv0 ); g_set_prgname( prgname ); g_free( prgname ); diff --git a/libvips/iofuncs/memory.c b/libvips/iofuncs/memory.c index f87c5475..ea8956dc 100644 --- a/libvips/iofuncs/memory.c +++ b/libvips/iofuncs/memory.c @@ -56,7 +56,6 @@ #include #include #include -#include #include #include @@ -75,6 +74,10 @@ * allocate and free memory. Most of VIPS uses them, though some parts use * the g_malloc() system instead, confusingly. * + * Use these functions for large allocations, such as arrays of image data. + * vips uses vips_alloc_get_mem(), which gives the amount of memory currently + * allocated via these functions, to decide when to start dropping cache. + * * If you compile with %DEBUGM it will track allocations for you, though * valgrind or dmalloc are better solutions. */ @@ -91,13 +94,14 @@ # warning DEBUG on in libsrc/iofuncs/memory.c #endif /*DEBUG*/ +static size_t vips_alloc_mem = 0; +static unsigned int vips_allocs = 0; +static size_t vips_alloc_mem_highwater = 0; +static GMutex *vips_alloc_mutex = NULL; + +#ifdef DEBUGM /* Track total alloc/total free here for debugging. */ -#ifdef DEBUGM -static size_t int total_mem_alloc = 0; -static unsigned int total_allocs = 0; -static size_t int high_water_mark = 0; -static GMutex *malloc_mutex = NULL; static GSList *malloc_list = NULL; static const int trace_freq = 100; /* Msg every this many malloc/free */ static int next_trace = 0; @@ -135,39 +139,43 @@ static int next_trace = 0; */ int vips_free( void *s ) -{ -#ifdef DEBUGM { size_t size; + /* Keep the size of the alloc in the previous 16 bytes. Ensures + * alignment rules are kept. + */ s = (void *) ((char*)s - 16); size = *((size_t*)s); - g_mutex_lock( malloc_mutex ); - assert( g_slist_find( malloc_list, s ) ); - malloc_list = g_slist_remove( malloc_list, s ); - assert( !g_slist_find( malloc_list, s ) ); - malloc_list = g_slist_remove( malloc_list, s ); - assert( total_allocs > 0 ); + g_mutex_lock( vips_alloc_mutex ); - total_mem_alloc -= size; - total_allocs -= 1; + if( vips_allocs <= 0 ) + vips_warn( "vips_malloc", + "%s", _( "vips_free: too many frees" ) ); + vips_alloc_mem -= size; + vips_allocs -= 1; + +#ifdef DEBUGM + g_assert( g_slist_find( malloc_list, s ) ); + malloc_list = g_slist_remove( malloc_list, s ); + g_assert( !g_slist_find( malloc_list, s ) ); + malloc_list = g_slist_remove( malloc_list, s ); next_trace += 1; if( next_trace > trace_freq ) { printf( "vips_free: %d, %d allocs, total %.3gM, " "high water %.3gM\n", size, - total_allocs, - total_mem_alloc / (1024.0 * 1024.0), - high_water_mark / (1024.0 * 1024.0) ); + vips_allocs, + vips_alloc_mem / (1024.0 * 1024.0), + vips_alloc_mem_highwater / (1024.0 * 1024.0) ); next_trace = 0; } - - g_mutex_unlock( malloc_mutex ); -} #endif /*DEBUGM*/ + g_mutex_unlock( vips_alloc_mutex ); + #ifdef DEBUG if( !s ) g_assert( 0 ); @@ -184,6 +192,14 @@ vips_malloc_cb( VipsImage *image, char *buf ) vips_free( buf ); } +/* g_mutex_new() is a macro. + */ +static void * +vips_alloc_mutex_new( void *data ) +{ + return( g_mutex_new() ); +} + /** * vips_malloc: * @image: allocate memory local to this #VipsImage, or %NULL @@ -203,22 +219,18 @@ vips_malloc_cb( VipsImage *image, char *buf ) void * vips_malloc( VipsImage *image, size_t size ) { + static GOnce vips_alloc_once = G_ONCE_INIT; + void *buf; -#ifdef DEBUGM - /* Assume the first vips_malloc() is single-threaded. - */ - if( !malloc_mutex ) - malloc_mutex = g_mutex_new(); -#endif /*DEBUGM*/ + vips_alloc_mutex = g_once( &vips_alloc_once, + vips_alloc_mutex_new, NULL ); -#ifdef DEBUGM - /* If debugging mallocs, need an extra sizeof(uint) bytes to track + /* Need an extra sizeof(size_t) bytes to track * size of this block. Ask for an extra 16 to make sure we don't break * alignment rules. */ size += 16; -#endif /*DEBUGM*/ if( !(buf = g_try_malloc( size )) ) { #ifdef DEBUG @@ -231,35 +243,41 @@ vips_malloc( VipsImage *image, size_t size ) vips_warn( "vips_malloc", _( "out of memory --- size == %dMB" ), (int) (size / (1024.0*1024.0)) ); + return( NULL ); } -#ifdef DEBUGM - /* Record number alloced. - */ - g_mutex_lock( malloc_mutex ); - assert( !g_slist_find( malloc_list, buf ) ); - malloc_list = g_slist_prepend( malloc_list, buf ); - *((size_t*)buf) = size; - buf = (void *) ((char*)buf + 16); - total_mem_alloc += size; - if( total_mem_alloc > high_water_mark ) - high_water_mark = total_mem_alloc; - total_allocs += 1; + g_mutex_lock( vips_alloc_mutex ); +#ifdef DEBUGM + g_assert( !g_slist_find( malloc_list, buf ) ); + malloc_list = g_slist_prepend( malloc_list, buf ); +#endif /*DEBUGM*/ + + *((size_t *)buf) = size; + buf = (void *) ((char *)buf + 16); + + vips_alloc_mem += size; + if( vips_alloc_mem > vips_alloc_mem_highwater ) + vips_alloc_mem_highwater = vips_alloc_mem; + vips_allocs += 1; + +#ifdef DEBUGM next_trace += 1; if( next_trace > trace_freq ) { printf( "vips_malloc: %d, %d allocs, total %.3gM, " "high water %.3gM\n", size, - total_allocs, - total_mem_alloc / (1024.0 * 1024.0), - high_water_mark / (1024.0 * 1024.0) ); + vips_allocs, + vips_alloc_mem / (1024.0 * 1024.0), + vips_alloc_mem_highwater / (1024.0 * 1024.0) ); next_trace = 0; } +#endif /*DEBUGM*/ - g_mutex_unlock( malloc_mutex ); + g_mutex_unlock( vips_alloc_mutex ); +#ifdef DEBUGM /* Handy to breakpoint on this printf() for catching large mallocs(). */ if( size > 1000000 ) @@ -287,3 +305,46 @@ vips_strdup( VipsImage *image, const char *str ) return( buf ); } + +/** + * vips_alloc_get_mem: + * + * Returns the number of bytes currently allocated via vips_malloc() and + * friends. vips uses this figure to decide when to start dropping cache, see + * #VipsOperation. + * + * Returns: the number of currently allocated bytes + */ +size_t +vips_alloc_get_mem( void ) +{ + return( vips_alloc_mem ); +} + +/** + * vips_alloc_get_mem_highwater: + * + * Returns the largest number of bytes simultaneously allocated via + * vips_malloc() and friends. + * + * Returns: the largest number of currently allocated bytes + */ +size_t +vips_alloc_get_mem_highwater( void ) +{ + return( vips_alloc_mem_highwater ); +} + +/** + * vips_alloc_get_allocs: + * + * Returns the number active allocations. + * + * Returns: the number active allocations + */ +unsigned int +vips_alloc_get_allocs( void ) +{ + return( vips_allocs ); +} + diff --git a/libvips/iofuncs/operation.c b/libvips/iofuncs/operation.c index abfae76d..de51b392 100644 --- a/libvips/iofuncs/operation.c +++ b/libvips/iofuncs/operation.c @@ -492,7 +492,7 @@ vips_call_required_optional( VipsOperation **operation, /* Build from cache. */ - if( vips_object_build_cache( (VipsObject **) operation ) ) + if( vips_operation_build_cache( operation ) ) return( -1 ); /* Walk args again, writing output.