From 594af28c8e4b6d0ba8825fd2a18f9527955f6418 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Tue, 25 Feb 2014 12:28:36 +0000
Subject: [PATCH] remove support for seq mode read for extract etc.

see comment in sequential

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.
---
 ChangeLog                       |  1 +
 libvips/conversion/extract.c    |  2 --
 libvips/conversion/ifthenelse.c |  2 --
 libvips/conversion/insert.c     |  2 --
 libvips/conversion/join.c       |  2 --
 libvips/conversion/sequential.c | 21 ++++++++++++++++-----
 6 files changed, 17 insertions(+), 13 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index a0accaef..d9ce58cc 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,6 @@
 24/2/14 started 7.38.5
 - jpeg load from buffer could write to input, thanks Lovell
+- remove support for seq mode read for operations like extract
 
 13/2/14 started 7.38.4
 - --sharpen=none option to vipsthumbnail was broken, thanks ferryfax
diff --git a/libvips/conversion/extract.c b/libvips/conversion/extract.c
index aab6c774..f586008f 100644
--- a/libvips/conversion/extract.c
+++ b/libvips/conversion/extract.c
@@ -193,8 +193,6 @@ 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" ),
diff --git a/libvips/conversion/ifthenelse.c b/libvips/conversion/ifthenelse.c
index 7c088e77..dcc2c990 100644
--- a/libvips/conversion/ifthenelse.c
+++ b/libvips/conversion/ifthenelse.c
@@ -470,8 +470,6 @@ vips_ifthenelse_class_init( VipsIfthenelseClass *class )
 	vobject_class->description = _( "ifthenelse an image" );
 	vobject_class->build = vips_ifthenelse_build;
 
-	operation_class->flags = VIPS_OPERATION_SEQUENTIAL_UNBUFFERED;
-
 	VIPS_ARG_IMAGE( class, "cond", -2, 
 		_( "Condition" ), 
 		_( "Condition input image" ),
diff --git a/libvips/conversion/insert.c b/libvips/conversion/insert.c
index f0c3a15a..a27ea28a 100644
--- a/libvips/conversion/insert.c
+++ b/libvips/conversion/insert.c
@@ -354,8 +354,6 @@ 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" ),
diff --git a/libvips/conversion/join.c b/libvips/conversion/join.c
index ad1ab9e3..af3f1398 100644
--- a/libvips/conversion/join.c
+++ b/libvips/conversion/join.c
@@ -228,8 +228,6 @@ vips_join_class_init( VipsJoinClass *class )
 	vobject_class->description = _( "join a pair of images" );
 	vobject_class->build = vips_join_build;
 
-	operation_class->flags = VIPS_OPERATION_SEQUENTIAL_UNBUFFERED;
-
 	VIPS_ARG_IMAGE( class, "in1", -1, 
 		_( "in1" ), 
 		_( "First input image" ),
diff --git a/libvips/conversion/sequential.c b/libvips/conversion/sequential.c
index 5f6def82..628464be 100644
--- a/libvips/conversion/sequential.c
+++ b/libvips/conversion/sequential.c
@@ -16,6 +16,9 @@
  * 4/9/12
  * 	- stop all threads on error
  * 	- don't stall forever, just delay ahead threads
+ * 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
  */
 
 /*
@@ -144,11 +147,9 @@ vips_sequential_generate( VipsRegion *or,
 		return( -1 );
 	}
 
-	if( r->top > sequential->y_pos /* &&
-		sequential->y_pos > 0 */ ) {
-		/* We have started reading (y_pos > 0) and this request is for
-		 * stuff beyond that, stall for a while to give other
-		 * threads time to catch up.
+	if( r->top > sequential->y_pos ) {
+		/* This request is for stuff beyond the current read position, 
+		 * stall for a while to give other threads time to catch up.
 		 * 
 		 * The stall can be cancelled by a signal on @ready.
 		 *
@@ -156,6 +157,16 @@ vips_sequential_generate( VipsRegion *or,
 		 * deadlock, and we don't fail on timeout, since the timeout 
 		 * 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;