From e7ae10ef68f43917c5db6ce3ee2bee978fd48998 Mon Sep 17 00:00:00 2001 From: John Cupitt Date: Mon, 27 Feb 2017 10:25:41 +0000 Subject: [PATCH] Revert "more UNBUF fixes" This reverts commit a4d3c2a7545f46582f4fc27bc543bf28d26e76ef. --- TODO | 8 +++++++- libvips/conversion/sequential.c | 1 - libvips/conversion/tilecache.c | 1 - libvips/include/vips/image.h | 5 ++++- libvips/iofuncs/enumtypes.c | 1 - libvips/iofuncs/object.c | 25 +++++++------------------ libvips/iofuncs/threadpool.c | 15 ++++++++++++++- libvips/resample/reduceh.cpp | 2 +- libvips/resample/reducev.cpp | 2 +- libvips/resample/thumbnail.c | 4 ++++ 10 files changed, 38 insertions(+), 26 deletions(-) diff --git a/TODO b/TODO index 391cbae1..a9b50dfc 100644 --- a/TODO +++ b/TODO @@ -12,9 +12,15 @@ same for fatstrip ... would it be less than tile-height? + deprecate UNBUFFERED? it will probably not work now + + remove single-thread-first-tile thing + the readbehind option to many loaders is no longer needed - check pforeign.h + do we need to restore the UNBUFFERED enum member to keep binary compat? we + need to make sure old code using UNBUFF now uses regular SEQ (and not RAND) + - vips linecache has access there twice! diff --git a/libvips/conversion/sequential.c b/libvips/conversion/sequential.c index d5a9a83c..c4412eeb 100644 --- a/libvips/conversion/sequential.c +++ b/libvips/conversion/sequential.c @@ -262,7 +262,6 @@ vips_sequential_init( VipsSequential *sequential ) sequential->lock = vips_g_mutex_new(); sequential->tile_height = 1; sequential->error = 0; - sequential->access = VIPS_ACCESS_SEQUENTIAL; } /** diff --git a/libvips/conversion/tilecache.c b/libvips/conversion/tilecache.c index 14808159..8fb979b0 100644 --- a/libvips/conversion/tilecache.c +++ b/libvips/conversion/tilecache.c @@ -280,7 +280,6 @@ vips_tile_search_recycle( gpointer key, gpointer value, gpointer user_data ) break; case VIPS_ACCESS_SEQUENTIAL: - case VIPS_ACCESS_SEQUENTIAL_UNBUFFERED: if( tile->pos.top < search->topmost ) { search->topmost = tile->pos.top; search->tile = tile; diff --git a/libvips/include/vips/image.h b/libvips/include/vips/image.h index 3f9fe380..f45da03a 100644 --- a/libvips/include/vips/image.h +++ b/libvips/include/vips/image.h @@ -129,10 +129,13 @@ typedef enum { typedef enum { VIPS_ACCESS_RANDOM, VIPS_ACCESS_SEQUENTIAL, - VIPS_ACCESS_SEQUENTIAL_UNBUFFERED, VIPS_ACCESS_LAST } VipsAccess; +/* Compat for a mode we used to support. + */ +#define VIPS_ACCESS_SEQUENTIAL_UNBUFFERED VIPS_ACCESS_SEQUENTIAL + struct _VipsImage; struct _VipsRegion; diff --git a/libvips/iofuncs/enumtypes.c b/libvips/iofuncs/enumtypes.c index d89595ef..537d3c2f 100644 --- a/libvips/iofuncs/enumtypes.c +++ b/libvips/iofuncs/enumtypes.c @@ -669,7 +669,6 @@ vips_access_get_type( void ) static const GEnumValue values[] = { {VIPS_ACCESS_RANDOM, "VIPS_ACCESS_RANDOM", "random"}, {VIPS_ACCESS_SEQUENTIAL, "VIPS_ACCESS_SEQUENTIAL", "sequential"}, - {VIPS_ACCESS_SEQUENTIAL_UNBUFFERED, "VIPS_ACCESS_SEQUENTIAL_UNBUFFERED", "sequential-unbuffered"}, {VIPS_ACCESS_LAST, "VIPS_ACCESS_LAST", "last"}, {0, NULL, NULL} }; diff --git a/libvips/iofuncs/object.c b/libvips/iofuncs/object.c index cbb5e8af..00ba0260 100644 --- a/libvips/iofuncs/object.c +++ b/libvips/iofuncs/object.c @@ -1816,7 +1816,6 @@ vips_object_set_argument_from_string( VipsObject *object, if( g_type_is_a( otype, VIPS_TYPE_IMAGE ) ) { VipsImage *out; VipsOperationFlags flags; - VipsAccess access; if( !value ) { vips_object_no_value( object, name ); @@ -1828,15 +1827,12 @@ vips_object_set_argument_from_string( VipsObject *object, flags = vips_operation_get_flags( VIPS_OPERATION( object ) ); - if( flags & - (VIPS_OPERATION_SEQUENTIAL | - VIPS_OPERATION_SEQUENTIAL_UNBUFFERED) ) - access = VIPS_ACCESS_SEQUENTIAL; - else - access = VIPS_ACCESS_RANDOM; + /* Read the filename. + */ if( !(out = vips_image_new_from_file( value, - "access", access, + "access", flags & VIPS_OPERATION_SEQUENTIAL ? + VIPS_ACCESS_SEQUENTIAL : VIPS_ACCESS_RANDOM, NULL )) ) return( -1 ); @@ -1855,7 +1851,6 @@ vips_object_set_argument_from_string( VipsObject *object, */ VipsArrayImage *array_image; VipsOperationFlags flags; - VipsAccess access; if( !value ) { vips_object_no_value( object, name ); @@ -1867,15 +1862,9 @@ vips_object_set_argument_from_string( VipsObject *object, flags = vips_operation_get_flags( VIPS_OPERATION( object ) ); - if( flags & - (VIPS_OPERATION_SEQUENTIAL | - VIPS_OPERATION_SEQUENTIAL_UNBUFFERED) ) - access = VIPS_ACCESS_SEQUENTIAL; - else - access = VIPS_ACCESS_RANDOM; - - if( !(array_image = - vips_array_image_new_from_string( value, access )) ) + if( !(array_image = vips_array_image_new_from_string( value, + flags & VIPS_OPERATION_SEQUENTIAL ? + VIPS_ACCESS_SEQUENTIAL : VIPS_ACCESS_RANDOM )) ) return( -1 ); g_value_init( &gvalue, VIPS_TYPE_ARRAY_IMAGE ); diff --git a/libvips/iofuncs/threadpool.c b/libvips/iofuncs/threadpool.c index c93060f0..68640805 100644 --- a/libvips/iofuncs/threadpool.c +++ b/libvips/iofuncs/threadpool.c @@ -548,6 +548,12 @@ typedef struct _VipsThreadpool { /* Set by Allocate (via an arg) to indicate normal end of computation. */ gboolean stop; + + /* Set by the first thread to hit allocate. The first work unit runs + * single-threaded to give loaders a change to get to the right spot + * in the input. + */ + gboolean done_first; } VipsThreadpool; /* Junk a thread. @@ -629,7 +635,8 @@ vips_thread_work_unit( VipsThread *thr ) return; } - g_mutex_unlock( pool->allocate_lock ); + if( pool->done_first ) + g_mutex_unlock( pool->allocate_lock ); /* Process a work unit. */ @@ -637,6 +644,11 @@ vips_thread_work_unit( VipsThread *thr ) thr->error = TRUE; pool->error = TRUE; } + + if( !pool->done_first ) { + pool->done_first = TRUE; + g_mutex_unlock( pool->allocate_lock ); + } } /* What runs as a thread ... loop, waiting to be told to do stuff. @@ -769,6 +781,7 @@ vips_threadpool_new( VipsImage *im ) vips_semaphore_init( &pool->tick, 0, "tick" ); pool->error = FALSE; pool->stop = FALSE; + pool->done_first = FALSE; /* If this is a tiny image, we won't need all nthr threads. Guess how * many tiles we might need to cover the image and use that to limit diff --git a/libvips/resample/reduceh.cpp b/libvips/resample/reduceh.cpp index 0dec0e0b..b57c4fcc 100644 --- a/libvips/resample/reduceh.cpp +++ b/libvips/resample/reduceh.cpp @@ -562,7 +562,7 @@ vips_reduceh_class_init( VipsReducehClass *reduceh_class ) vobject_class->description = _( "shrink an image horizontally" ); vobject_class->build = vips_reduceh_build; - operation_class->flags = VIPS_OPERATION_SEQUENTIAL; + operation_class->flags = VIPS_OPERATION_SEQUENTIAL_UNBUFFERED; VIPS_ARG_DOUBLE( reduceh_class, "hshrink", 3, _( "Hshrink" ), diff --git a/libvips/resample/reducev.cpp b/libvips/resample/reducev.cpp index 3f7400cb..3cf3d7f7 100644 --- a/libvips/resample/reducev.cpp +++ b/libvips/resample/reducev.cpp @@ -895,7 +895,7 @@ vips_reducev_class_init( VipsReducevClass *reducev_class ) vobject_class->description = _( "shrink an image vertically" ); vobject_class->build = vips_reducev_build; - operation_class->flags = VIPS_OPERATION_SEQUENTIAL; + operation_class->flags = VIPS_OPERATION_SEQUENTIAL_UNBUFFERED; VIPS_ARG_DOUBLE( reducev_class, "vshrink", 3, _( "Vshrink" ), diff --git a/libvips/resample/thumbnail.c b/libvips/resample/thumbnail.c index 511adc68..14d30447 100644 --- a/libvips/resample/thumbnail.c +++ b/libvips/resample/thumbnail.c @@ -634,6 +634,8 @@ vips_thumbnail_file_open( VipsThumbnail *thumbnail, int shrink, double scale ) { VipsThumbnailFile *file = (VipsThumbnailFile *) thumbnail; + /* We can't use UNBUFERRED safely on very-many-core systems. + */ if( shrink != 1 ) return( vips_image_new_from_file( file->filename, "access", VIPS_ACCESS_SEQUENTIAL, @@ -796,6 +798,8 @@ vips_thumbnail_buffer_open( VipsThumbnail *thumbnail, { VipsThumbnailBuffer *buffer = (VipsThumbnailBuffer *) thumbnail; + /* We can't use UNBUFERRED safely on very-many-core systems. + */ if( shrink != 1 ) return( vips_image_new_from_buffer( buffer->buf->data, buffer->buf->length, "",