skipahead is back

thanks to a new threadpool idea, see

https://github.com/jcupitt/libvips/issues/117

ChangeLog

@@ -37,6 +37,7 @@
   im_*mosaic1(), im_*merge1() redone as classes
 - better filename tracking for globalbalance
 - revised vips8 image load/save API, now simpler and more logical
+- skipahead is back, thanks to a new threadpool tweak
 
 6/3/14 started 7.38.6
 - grey ramp minimum was wrong

TODO

@@ -1,3 +1,7 @@
+- note we have broken binary compat
+
+- change nip2 examples ws versions
+
 
 - can we use postbuild elsewhere? look at use of "preclose" / "written", etc. 
 
@@ -23,39 +27,6 @@
 
 
 
-- think of a better way to support skipahead
-
-  extract needs to hint to it's image sources what line it will start reading
-  at 
-
-  in vips_extract_build(), call vips_image_set_start(), somehow sends a signal
-  the wrong way down the pipe to the vips_sequential() (if there is one)
-  telling it not to stall the first request if it's for that line
-   
-  test --crop in vipsthumbnail on portrait images cropped to landscape
-
-  alternative: if extract sees a seq image, it deliberately reads and discards
-  the N scanlines 
-
-  	could be very slow :-( 
-
-  the first vips_extract_area_gen() needs to somehow signal in
-  vips_region_prepare() that this is the first request
-
-  	how do we pass the signal down? not clear
-
-  run the first tile single-threaded
-
-  	so sinkdisc runs the first request, waits for it to come back, then
-	starts all the workers going
-
-	this is quite a good idea! we'd not slow down much, and there's a huge
-	amount of locking on the first tile anyway
-
-	now seq can support skipahead again
-
-
-
 
 
 

libvips/conversion/extract.c

@@ -182,6 +182,7 @@ vips_extract_area_class_init( VipsExtractAreaClass *class )
 {
 	GObjectClass *gobject_class = G_OBJECT_CLASS( class );
 	VipsObjectClass *vobject_class = VIPS_OBJECT_CLASS( class );
+	VipsOperationClass *operation_class = VIPS_OPERATION_CLASS( class );
 
 	VIPS_DEBUG_MSG( "vips_extract_area_class_init\n" );
 
@@ -192,6 +193,8 @@ vips_extract_area_class_init( VipsExtractAreaClass *class )
 	vobject_class->description = _( "extract an area from an image" );
 	vobject_class->build = vips_extract_area_build;
 
+	operation_class->flags = VIPS_OPERATION_SEQUENTIAL_UNBUFFERED;
+
 	VIPS_ARG_IMAGE( class, "input", 0, 
 		_( "Input" ), 
 		_( "Input image" ),

libvips/conversion/insert.c

@@ -443,6 +443,7 @@ vips_insert_class_init( VipsInsertClass *class )
 {
 	GObjectClass *gobject_class = G_OBJECT_CLASS( class );
 	VipsObjectClass *vobject_class = VIPS_OBJECT_CLASS( class );
+	VipsOperationClass *operation_class = VIPS_OPERATION_CLASS( class );
 
 	VIPS_DEBUG_MSG( "vips_insert_class_init\n" );
 
@@ -453,6 +454,8 @@ vips_insert_class_init( VipsInsertClass *class )
 	vobject_class->description = _( "insert an image" );
 	vobject_class->build = vips_insert_build;
 
+	operation_class->flags = VIPS_OPERATION_SEQUENTIAL_UNBUFFERED;
+
 	VIPS_ARG_IMAGE( class, "main", -1, 
 		_( "Main" ), 
 		_( "Main input image" ),

libvips/conversion/sequential.c

@@ -19,6 +19,8 @@
  * 25/2/14
  * 	- we were allowing skipahead if the first request was for y>0, but
  * 	  this broke on some busy, many-core systems, see comment below
+ * 10/6/14
+ * 	- re-enable skipahead now we have the single-thread-first-tile idea
  */
 
 /*
@@ -149,8 +151,10 @@ vips_sequential_generate( VipsRegion *or,
 		return( -1 );
 	}
 
-	if( r->top > sequential->y_pos ) {
+	if( r->top > sequential->y_pos &&
+		sequential->y_pos > 0 ) {
 		/* This request is for stuff beyond the current read position, 
+		 * and this is not the first request. We 
 		 * stall for a while to give other threads time to catch up.
 		 * 
 		 * The stall can be cancelled by a signal on @ready.
@@ -160,15 +164,6 @@ vips_sequential_generate( VipsRegion *or,
 		 * may be harmless.
 		 */
 
-		/* We used to not stall if the read position was zero, ie. if
-		 * the first request was for a line some way down the image,
-		 * and assume this was extract or somesuch. But this could
-		 * sometimes break on busy, many-core systems.
-		 *
-		 * Think of a better way to support eg. extract safely in
-		 * sequential mode.
-		 */
-
 #ifdef HAVE_COND_INIT
 		gint64 time;
 

libvips/iofuncs/threadpool.c

@@ -493,6 +493,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.
@@ -534,8 +540,11 @@ vips_thread_allocate( VipsThread *thr )
 	return( 0 );
 }
 
-/* The main loop: get some work, do it! Can run from many worker threads, or 
+/* Run this once per main loop. Get some work (single-threaded), then do it
- * from the main thread if threading is off. 
+ * (many-threaded).
+ *
+ * The very first workunit is also executed single-threaded. This gives
+ * loaders a change to seek to the correct spot, see vips_sequential().
  */
 static void
 vips_thread_work_unit( VipsThread *thr )
@@ -572,7 +581,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.
 	 */
@@ -580,6 +590,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.
@@ -603,7 +618,8 @@ vips_thread_main_loop( void *a )
 		VIPS_GATE_STOP( "vips_thread_work_unit: u" ); 
 		vips_semaphore_up( &pool->tick );
 
-		if( pool->stop || pool->error )
+		if( pool->stop || 
+			pool->error )
 			break;
 	} 
 
@@ -703,8 +719,9 @@ vips_threadpool_new( VipsImage *im )
 	pool->thr = NULL;
 	vips_semaphore_init( &pool->finish, 0, "finish" );
 	vips_semaphore_init( &pool->tick, 0, "tick" );
-	pool->stop = FALSE;
 	pool->error = FALSE;
+	pool->stop = FALSE;
+	pool->done_first = FALSE;
 
 	/* Attach tidy-up callback.
 	 */
@@ -887,7 +904,8 @@ vips_threadpool_run( VipsImage *im,
 			progress( pool->a ) ) 
 			pool->error = TRUE;
 
-		if( pool->stop || pool->error )
+		if( pool->stop || 
+			pool->error )
 			break;
 	}