From 52c8678b23d2c4c4ecd4b7434f9bf1a4280acaf3 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Wed, 2 Nov 2011 14:51:39 +0000 Subject: [PATCH] flush on too many open files start to drop cached operations when there are too many files open also, CLI args to control the cache limit, and more informative --vips-leak messages --- ChangeLog | 2 + TODO | 19 ++-- libvips/include/vips/buf.h | 1 + libvips/include/vips/internal.h | 1 + libvips/include/vips/memory.h | 4 + libvips/include/vips/operation.h | 2 + libvips/iofuncs/buf.c | 49 ++++++++++ libvips/iofuncs/cache.c | 41 ++++++++ libvips/iofuncs/image.c | 2 +- libvips/iofuncs/init.c | 28 ++++-- libvips/iofuncs/memory.c | 157 ++++++++++++++++++++++++++++--- libvips/iofuncs/util.c | 1 - libvips/iofuncs/vips.c | 16 ++-- 13 files changed, 289 insertions(+), 34 deletions(-) diff --git a/ChangeLog b/ChangeLog index 7000264f..8c9d85e1 100644 --- a/ChangeLog +++ b/ChangeLog @@ -29,6 +29,8 @@ - added array members and arguments - added nary - remove VipsPool, vips_object_local_array() is much better +- cache.c now drops if you have too many open files +- CLI args to change max files 12/10/11 started 7.26.6 - NOCACHE was not being set correctly on OS X causing performance diff --git a/TODO b/TODO index 32131197..2512d16e 100644 --- a/TODO +++ b/TODO @@ -7,17 +7,10 @@ this doesn't work, I wonder why ... do we need to add an extra ref to t? + yes, the old obj holds a ref to the operation, not the other way around -- in cache.c, we must not include the hash of the output objects, I guess we - don't - - - -- get rid of vipspool - - - vipsimage should be cached too, eg. @@ -26,6 +19,12 @@ can be reused + ah ha! will im_jpeg2vips() or whatever get cached? that would do the reuse + for us + + move format to new-style and try it + + @@ -44,6 +43,8 @@ should boxed get freed in finalise rather than dispose? + vipsobject has few docs atm :( + @@ -59,6 +60,8 @@ use array version in several places + things like black and min (sources and sinks) only set the dhint + diff --git a/libvips/include/vips/buf.h b/libvips/include/vips/buf.h index fda8f38f..e0328d6a 100644 --- a/libvips/include/vips/buf.h +++ b/libvips/include/vips/buf.h @@ -69,6 +69,7 @@ gboolean vips_buf_vappendf( VipsBuf *buf, const char *fmt, va_list ap ); gboolean vips_buf_appendc( VipsBuf *buf, char ch ); gboolean vips_buf_appendsc( VipsBuf *buf, gboolean quote, const char *str ); gboolean vips_buf_appendgv( VipsBuf *buf, GValue *value ); +gboolean vips_buf_append_size( VipsBuf *buf, size_t n ); gboolean vips_buf_removec( VipsBuf *buf, char ch ); gboolean vips_buf_change( VipsBuf *buf, const char *o, const char *n ); gboolean vips_buf_is_empty( VipsBuf *buf ); diff --git a/libvips/include/vips/internal.h b/libvips/include/vips/internal.h index 5d9905cc..7cc16db7 100644 --- a/libvips/include/vips/internal.h +++ b/libvips/include/vips/internal.h @@ -79,6 +79,7 @@ extern char *vips__disc_threshold; */ extern char *vips__cache_max; extern char *vips__cache_max_mem; +extern char *vips__cache_max_files; typedef int (*im__fftproc_fn)( VipsImage *, VipsImage *, VipsImage * ); diff --git a/libvips/include/vips/memory.h b/libvips/include/vips/memory.h index da3f3b4c..1f35bc19 100644 --- a/libvips/include/vips/memory.h +++ b/libvips/include/vips/memory.h @@ -74,6 +74,10 @@ size_t vips_tracked_get_mem( void ); size_t vips_tracked_get_mem_highwater( void ); int vips_tracked_get_allocs( void ); +int vips_tracked_open( const char *pathname, int flags, ... ); +int vips_tracked_close( int fd ); +int vips_tracked_get_files( void ); + #ifdef __cplusplus } #endif /*__cplusplus*/ diff --git a/libvips/include/vips/operation.h b/libvips/include/vips/operation.h index 67b3b19e..3ba62869 100644 --- a/libvips/include/vips/operation.h +++ b/libvips/include/vips/operation.h @@ -88,6 +88,8 @@ void vips_cache_set_max_mem( int max_mem ); int vips_cache_get_max( void ); int vips_cache_get_size( void ); size_t vips_cache_get_max_mem( void ); +int vips_cache_get_max_files( void ); +void vips_cache_set_max_files( int max_files ); #ifdef __cplusplus } diff --git a/libvips/iofuncs/buf.c b/libvips/iofuncs/buf.c index 1048a250..815fb76c 100644 --- a/libvips/iofuncs/buf.c +++ b/libvips/iofuncs/buf.c @@ -501,6 +501,55 @@ vips_buf_appendgv( VipsBuf *buf, GValue *value ) return( result ); } +/** + * vips_buf_append_size: + * @buf: the buffer + * @n: the number of bytes + * + * Turn a number of bytes into a sensible string ... eg "12", "12KB", "12MB", + * "12GB" etc. + * + * Returns: %FALSE on overflow, %TRUE otherwise. + */ +gboolean +vips_buf_append_size( VipsBuf *buf, size_t n ) +{ + const static char *names[] = { + /* File length unit. + */ + N_( "bytes" ), + + /* Kilo byte unit. + */ + N_( "KB" ), + + /* Mega byte unit. + */ + N_( "MB" ), + + /* Giga byte unit. + */ + N_( "GB" ), + + /* Tera byte unit. + */ + N_( "TB" ) + }; + + double sz = n; + int i; + + for( i = 0; sz > 1024 && i < VIPS_NUMBER( names ); sz /= 1024, i++ ) + ; + + if( i == 0 ) + /* No decimal places for bytes. + */ + return( vips_buf_appendf( buf, "%g %s", sz, _( names[i] ) ) ); + else + return( vips_buf_appendf( buf, "%.2f %s", sz, _( names[i] ) ) ); +} + /** * vips_buf_all: * @buf: the buffer diff --git a/libvips/iofuncs/cache.c b/libvips/iofuncs/cache.c index 1a9c6502..19c9f25e 100644 --- a/libvips/iofuncs/cache.c +++ b/libvips/iofuncs/cache.c @@ -70,11 +70,16 @@ */ char *vips__cache_max = NULL; char *vips__cache_max_mem = NULL; +char *vips__cache_max_files = NULL; /* Max number of cached operations. */ static int vips_cache_max = 10000; +/* How many tracked open files we allow before we start dropping cache. + */ +static int vips_cache_max_files = 900; + /* How much RAM we spend on caches before we start dropping cached operations * ... default 1gb. */ @@ -397,6 +402,10 @@ vips_cache_init( void ) if( vips__cache_max_mem ) vips_cache_max_mem = vips__parse_size( vips__cache_max_mem ); + + if( vips__cache_max_files ) + vips_cache_max_files = + vips__parse_size( vips__cache_max_files ); } } @@ -507,6 +516,7 @@ vips_cache_trim( void ) VipsOperation *operation; while( (g_hash_table_size( vips_cache_table ) > vips_cache_max || + vips_tracked_get_files() > vips_cache_max_files || vips_tracked_get_mem() > vips_cache_max_mem) && (operation = vips_cache_select()) ) vips_cache_drop( operation ); @@ -669,3 +679,34 @@ vips_cache_get_max_mem( void ) { return( vips_cache_max_mem ); } + +/** + * vips_cache_get_max_files: + * + * Get the maximum number of tracked files we allow before we start dropping + * cached operations. See vips_tracked_get_files(). + * + * See also: vips_tracked_get_files(). + * + * Returns: the maximum number of tracked files we allow + */ +int +vips_cache_get_max_files( void ) +{ + return( vips_cache_max_files ); +} + +/** + * vips_cache_set_max_files: + * + * Set the maximum number of tracked files we allow before we start dropping + * cached operations. See vips_tracked_get_files(). + * + * See also: vips_tracked_get_files(). + */ +void +vips_cache_set_max_files( int max_files ) +{ + vips_cache_max_files = max_files; + vips_cache_trim(); +} diff --git a/libvips/iofuncs/image.c b/libvips/iofuncs/image.c index 69f4f01d..a7892eae 100644 --- a/libvips/iofuncs/image.c +++ b/libvips/iofuncs/image.c @@ -224,7 +224,7 @@ vips_image_finalize( GObject *gobject ) if( image->dtype == VIPS_IMAGE_OPENOUT ) (void) vips__writehist( image ); - if( close( image->fd ) == -1 ) + if( vips_tracked_close( image->fd ) == -1 ) vips_error( "VipsImage", "%s", _( "unable to close fd" ) ); image->fd = -1; diff --git a/libvips/iofuncs/init.c b/libvips/iofuncs/init.c index 6eb2f081..96e0400e 100644 --- a/libvips/iofuncs/init.c +++ b/libvips/iofuncs/init.c @@ -290,17 +290,30 @@ void vips_shutdown( void ) { vips_cache_drop_all(); + im_close_plugins(); + /* In dev releases, always show leaks. + */ +#ifndef DEBUG_LEAK if( vips__leak ) +#endif /*DEBUG_LEAK*/ + { + char txt[1024]; + VipsBuf buf = VIPS_BUF_STATIC( txt ); + vips_object_print_all(); -/* In dev releases, always show leaks. - */ -#ifdef DEBUG_LEAK - vips_object_print_all(); -#endif /*DEBUG_LEAK*/ + vips_buf_appendf( &buf, + "tracked memory: %d allocations totalling %zd bytes\n", + vips_tracked_get_allocs(), + vips_tracked_get_mem() ); + vips_buf_appendf( &buf, "tracked memory: high-water mark " ); + vips_buf_append_size( &buf, vips_tracked_get_mem_highwater() ); + vips_buf_appendf( &buf, "\ntracked files: %d open\n", + vips_tracked_get_files() ); - im_close_plugins(); + printf( "%s", vips_buf_all( &buf ) ); + } } const char * @@ -353,6 +366,9 @@ static GOptionEntry option_entries[] = { { "vips-cache-max-memory", 'm', 0, G_OPTION_ARG_STRING, &vips__cache_max_mem, N_( "cache at most N bytes in memory" ), "N" }, + { "vips-cache-max-files", 'l', 0, + G_OPTION_ARG_STRING, &vips__cache_max_files, + N_( "allow at most N open files" ), "N" }, { NULL } }; diff --git a/libvips/iofuncs/memory.c b/libvips/iofuncs/memory.c index e358ce91..728109ec 100644 --- a/libvips/iofuncs/memory.c +++ b/libvips/iofuncs/memory.c @@ -55,6 +55,10 @@ #endif /*HAVE_CONFIG_H*/ #include +#include +#include +#include +#include #include #include #include @@ -95,6 +99,7 @@ static int vips_tracked_allocs = 0; static size_t vips_tracked_mem = 0; +static size_t vips_tracked_files = 0; static size_t vips_tracked_mem_highwater = 0; static GMutex *vips_tracked_mutex = NULL; @@ -243,6 +248,15 @@ vips_tracked_mutex_new( void *data ) return( g_mutex_new() ); } +static void +vips_tracked_init( void ) +{ + static GOnce vips_tracked_once = G_ONCE_INIT; + + vips_tracked_mutex = g_once( &vips_tracked_once, + vips_tracked_mutex_new, NULL ); +} + /** * vips_tracked_malloc: * @size: number of bytes to allocate @@ -262,12 +276,9 @@ vips_tracked_mutex_new( void *data ) void * vips_tracked_malloc( size_t size ) { - static GOnce vips_tracked_once = G_ONCE_INIT; - void *buf; - vips_tracked_mutex = g_once( &vips_tracked_once, - vips_tracked_mutex_new, NULL ); + vips_tracked_init(); /* 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 @@ -306,7 +317,76 @@ vips_tracked_malloc( size_t size ) } /** - * vips_alloc_get_mem: + * vips_tracked_open: + * @pathname: name of file to open + * @flags: flags for open() + * @mode: open mode + * + * Exactly as open(2), but the number of files current open via + * vips_tracked_open() is available via vips_tracked_get_files(). This is used + * by the vips operation cache to drop cache when the number of files + * available is low. + * + * You must only close the file descriptor with vips_tracked_close(). + * + * See also: vips_tracked_close(), vips_tracked_get_files(). + * + * Returns: a file descriptor, or -1 on error. + */ +int +vips_tracked_open( const char *pathname, int flags, ... ) +{ + int fd; + mode_t mode; + va_list ap; + + va_start( ap, flags ); + mode = va_arg( ap, mode_t ); + va_end( ap ); + + if( (fd = open( pathname, flags, mode )) == -1 ) + return( -1 ); + + vips_tracked_init(); + + g_mutex_lock( vips_tracked_mutex ); + + vips_tracked_files += 1; + + g_mutex_unlock( vips_tracked_mutex ); + + return( fd ); +} + +/** + * vips_tracked_close: + * @fd: file to close() + * + * Exactly as close(2), but update the number of files currently open via + * vips_tracked_get_files(). This is used + * by the vips operation cache to drop cache when the number of files + * available is low. + * + * You must only close file descriptors opened with vips_tracked_open(). + * + * See also: vips_tracked_open(), vips_tracked_get_files(). + * + * Returns: a file descriptor, or -1 on error. + */ +int +vips_tracked_close( int fd ) +{ + g_mutex_lock( vips_tracked_mutex ); + + vips_tracked_files -= 1; + + g_mutex_unlock( vips_tracked_mutex ); + + return( close( fd ) ); +} + +/** + * vips_tracked_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 @@ -317,33 +397,88 @@ vips_tracked_malloc( size_t size ) size_t vips_tracked_get_mem( void ) { - return( vips_tracked_mem ); + size_t mem; + + vips_tracked_init(); + + g_mutex_lock( vips_tracked_mutex ); + + mem = vips_tracked_mem; + + g_mutex_unlock( vips_tracked_mutex ); + + return( mem ); } /** * vips_tracked_get_mem_highwater: * * Returns the largest number of bytes simultaneously allocated via - * vips_malloc() and friends. + * vips_tracked_malloc(). Handy for estimating max memory requirements for a + * program. * * Returns: the largest number of currently allocated bytes */ size_t vips_tracked_get_mem_highwater( void ) { - return( vips_tracked_mem_highwater ); + size_t mx; + + vips_tracked_init(); + + g_mutex_lock( vips_tracked_mutex ); + + mx = vips_tracked_mem_highwater; + + g_mutex_unlock( vips_tracked_mutex ); + + return( mx ); } /** * vips_tracked_get_allocs: * - * Returns the number active allocations. + * Returns the number of active allocations. * - * Returns: the number active allocations + * Returns: the number of active allocations */ int vips_tracked_get_allocs( void ) { - return( vips_tracked_allocs ); + int n; + + vips_tracked_init(); + + g_mutex_lock( vips_tracked_mutex ); + + n = vips_tracked_allocs; + + g_mutex_unlock( vips_tracked_mutex ); + + return( n ); +} + + +/** + * vips_tracked_get_files: + * + * Returns the number of open files. + * + * Returns: the number of open files + */ +int +vips_tracked_get_files( void ) +{ + int n; + + vips_tracked_init(); + + g_mutex_lock( vips_tracked_mutex ); + + n = vips_tracked_files; + + g_mutex_unlock( vips_tracked_mutex ); + + return( n ); } diff --git a/libvips/iofuncs/util.c b/libvips/iofuncs/util.c index 44b092df..7ab3611d 100644 --- a/libvips/iofuncs/util.c +++ b/libvips/iofuncs/util.c @@ -1494,4 +1494,3 @@ vips__parse_size( const char *size_string ) return( size ); } - diff --git a/libvips/iofuncs/vips.c b/libvips/iofuncs/vips.c index 02011e8f..ef145dc2 100644 --- a/libvips/iofuncs/vips.c +++ b/libvips/iofuncs/vips.c @@ -131,14 +131,16 @@ vips__open_image_read( const char *filename ) * work. When we later mmap this file, we set read-only, so there * is little danger of scrubbing over files we own. */ - if( (fd = open( filename, MODE_READWRITE )) == -1 ) { + fd = vips_tracked_open( filename, MODE_READWRITE ); + if( fd == -1 ) /* Open read-write failed. Fall back to open read-only. */ - if( (fd = open( filename, MODE_READONLY )) == -1 ) { - vips_error_system( errno, "VipsImage", - _( "unable to open \"%s\"" ), filename ); - return( -1 ); - } + fd = vips_tracked_open( filename, MODE_READONLY ); + + if( fd == -1 ) { + vips_error_system( errno, "VipsImage", + _( "unable to open \"%s\"" ), filename ); + return( -1 ); } return( fd ); @@ -951,7 +953,7 @@ vips_image_open_output( VipsImage *image ) */ unsigned char header[VIPS_SIZEOF_HEADER]; - if( (image->fd = open( image->filename, + if( (image->fd = vips_tracked_open( image->filename, MODE_WRITE, 0666 )) < 0 ) { vips_error_system( errno, "VipsImage", _( "unable to write to \"%s\"" ),