experiment with a different early-close strategy

We close loaders early in order to save file handles, and on Windows to
make sure that files can be deleted as soon as possible.

Currently loaders do this by watching the Y coordinate of requests and
freeing the fd when the final line of the file is fetched. This is messy
and does not always work, since there are cases when the final line is
not fetched.

Instead, this patch gets loaders to listen for "minimise" on their
output and close on that. This signal is emitted on all upstream images
whenever a threadpool finishes a scan of an image and is usually used to
trim caches after computation.

See https://github.com/libvips/libvips/issues/1370
This commit is contained in:
John Cupitt 2019-07-20 16:31:30 +01:00
parent 64a6a27326
commit 2c654060f9
5 changed files with 49 additions and 74 deletions

View File

@ -6,6 +6,7 @@
- disable webp alpha output if all frames fill the canvas and are solid
- add "compression" option to heifsave [lovell]
- support webp and zstd compression in tiff
- use "minimise" to close input early
9/7/19 started 8.8.2
- better early shutdown in readers

View File

@ -100,6 +100,8 @@
* - strict round down on shrink-on-load
* 16/8/18
* - shut down the input file as soon as we can [kleisauke]
* 20/7/19
* - close input on minimise rather than Y read position
*/
/*
@ -190,8 +192,7 @@ typedef struct _ReadJpeg {
int output_height;
} ReadJpeg;
/* This can be called many times. It's called directly at the end of image
* read.
/* This can be called many times.
*/
static void
readjpeg_close_input( ReadJpeg *jpeg )
@ -237,6 +238,12 @@ readjpeg_close_cb( VipsObject *object, ReadJpeg *jpeg )
(void) readjpeg_free( jpeg );
}
static void
readjpeg_minimise_cb( VipsObject *object, ReadJpeg *jpeg )
{
readjpeg_close_input( jpeg );
}
static ReadJpeg *
readjpeg_new( VipsImage *out, int shrink, gboolean fail, gboolean autorotate )
{
@ -270,6 +277,8 @@ readjpeg_new( VipsImage *out, int shrink, gboolean fail, gboolean autorotate )
g_signal_connect( out, "close",
G_CALLBACK( readjpeg_close_cb ), jpeg );
g_signal_connect( out, "minimise",
G_CALLBACK( readjpeg_minimise_cb ), jpeg );
return( jpeg );
}
@ -718,10 +727,6 @@ read_jpeg_generate( VipsRegion *or,
jpeg->y_pos += 1;
}
/* Shut down the input file as soon as we can.
*/
if( jpeg->y_pos >= or->im->Ysize )
readjpeg_close_input( jpeg );
VIPS_GATE_STOP( "read_jpeg_generate: work" );

View File

@ -189,6 +189,8 @@
* 7/6/19
* - istiff reads the first directory rather than just testing the magic
* number, so it ignores more TIFF-like, but not TIFF images
* 20/7/19
* - use "minimise" for early shutdown, rather than read Y position
*/
/*
@ -494,6 +496,20 @@ rtiff_close_cb( VipsObject *object, Rtiff *rtiff )
rtiff_free( rtiff );
}
static void
rtiff_minimise_cb( VipsObject *object, Rtiff *rtiff )
{
#ifdef DEBUG
printf( "rtiff_minimise_cb: %p minimise\n", rtiff );
#endif /*DEBUG*/
/* Close early for non-tiled TIFFs. Tiled TIFFs are read randomly, so
* the end of a loop doesn't mean the tiff won't be used again.
*/
if( !rtiff->header.tiled )
rtiff_free( rtiff );
}
static Rtiff *
rtiff_new( VipsImage *out, int page, int n, gboolean autorotate )
{
@ -520,6 +536,9 @@ rtiff_new( VipsImage *out, int page, int n, gboolean autorotate )
g_signal_connect( out, "close",
G_CALLBACK( rtiff_close_cb ), rtiff );
g_signal_connect( out, "minimise",
G_CALLBACK( rtiff_minimise_cb ), rtiff );
if( rtiff->page < 0 || rtiff->page > 1000000 ) {
vips_error( "tiff2vips", _( "bad page number %d" ),
rtiff->page );
@ -1603,11 +1622,6 @@ rtiff_fill_region( VipsRegion *out,
VIPS_GATE_STOP( "rtiff_fill_region: work" );
/* We can't shut down the input file early for tile read, even if we
* know load is in sequential mode, since we are not inside a
* vips_sequential() and requests are not guaranteed to be in order.
*/
return( 0 );
}
@ -1949,15 +1963,6 @@ rtiff_stripwise_generate( VipsRegion *or,
rtiff->y_pos += hit.height;
}
/* Shut down the input file as soon as we can.
*/
if( rtiff->y_pos >= or->im->Ysize ) {
#ifdef DEBUG
printf( "rtiff_stripwise_generate: early shutdown\n" );
#endif /*DEBUG*/
rtiff_free( rtiff );
}
VIPS_GATE_STOP( "rtiff_stripwise_generate: work" );
return( 0 );

View File

@ -196,6 +196,22 @@ read_close_cb( VipsImage *out, Read *read )
read_destroy( read );
}
static void
read_minimise_cb( VipsImage *out, Read *read )
{
/* Catch errors from png_read_end(). This can fail on a truncated
* file.
*/
if( read->pPng ) {
if( setjmp( png_jmpbuf( read->pPng ) ) )
return;
png_read_end( read->pPng, NULL );
}
read_destroy( read );
}
static Read *
read_new( VipsImage *out, gboolean fail )
{
@ -218,6 +234,8 @@ read_new( VipsImage *out, gboolean fail )
g_signal_connect( out, "close",
G_CALLBACK( read_close_cb ), read );
g_signal_connect( out, "minimise",
G_CALLBACK( read_minimise_cb ), read );
if( !(read->pPng = png_create_read_struct(
PNG_LIBPNG_VER_STRING, NULL,
@ -635,26 +653,6 @@ png2vips_generate( VipsRegion *or,
read->y_pos += 1;
}
/* Catch errors from png_read_end(). This can fail on a truncated
* file.
*/
if( setjmp( png_jmpbuf( read->pPng ) ) ) {
if( read->fail ) {
vips_error( "vipspng", "%s", _( "libpng read error" ) );
return( -1 );
}
return( 0 );
}
/* We need to shut down the reader immediately at the end of read or
* we won't detach ready for the next image.
*/
if( read->y_pos >= read->out->Ysize ) {
png_read_end( read->pPng, NULL );
read_destroy( read );
}
return( 0 );
}

View File

@ -45,8 +45,6 @@
* - rename yshrink -> vshrink for greater consistency
* 7/3/17
* - add a seq line cache
* 9/7/19
* - read the tail of the input to force early shutdown in seq readers
*/
/*
@ -319,38 +317,6 @@ vips_shrinkv_gen( VipsRegion *or, void *vseq,
VIPS_COUNT_PIXELS( or, "vips_shrinkv_gen" );
/* If we are in seq mode and we've just generated the last line of
* the output, make sure we read all of the input.
*
* This will trigger the early shutdown logic in things like the
* tiff loader.
*/
if( shrink->sequential &&
r->top + r->height >= or->im->Ysize ) {
/* First unused scanline. resample->in->Ysize because we want
* the height before the embed.
*/
int first = or->im->Ysize * shrink->vshrink;
int unused = resample->in->Ysize - first;
g_assert( unused >= 0 );
for( y = 0; y < unused; y++ ) {
VipsRect s;
s.left = r->left;
s.top = first + y;
s.width = r->width;
s.height = 1;
#ifdef DEBUG
printf( "shrink_gen: requesting tail %d\n", s.top );
#endif /*DEBUG*/
if( vips_region_prepare( ir, &s ) )
return( -1 );
}
}
return( 0 );
}