From e01a90f7cfefbe10dce5e4f3f488f78fc75bbc73 Mon Sep 17 00:00:00 2001
From: John Cupitt <jcupitt@gmail.com>
Date: Thu, 13 Apr 2017 16:24:51 +0100
Subject: [PATCH] revise cache sizing

we had output buffers too large, input caches too small

see https://github.com/jcupitt/libvips/issues/639
---
 ChangeLog                      |  2 ++
 TODO                           |  2 --
 libvips/conversion/tilecache.c |  7 ++++++-
 libvips/iofuncs/threadpool.c   | 13 +++++++++----
 4 files changed, 17 insertions(+), 7 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 8f06ed74..fa84c42c 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,7 @@
 7/4/17 started 8.5.3
 - more link fixing in docs 
+- revise cache sizing again to help out of order errors under heavy load, thanks
+  kleisauke
 
 25/3/17 started 8.5.2
 - better behaviour for truncated PNG files, thanks Yury
diff --git a/TODO b/TODO
index 7aa1f7ac..36f83c6e 100644
--- a/TODO
+++ b/TODO
@@ -1,5 +1,3 @@
-- add analytics tags to docs output
-
 - not sure about utf8 error messages on win
 
 - strange: 
diff --git a/libvips/conversion/tilecache.c b/libvips/conversion/tilecache.c
index 9f1d4995..9d78a726 100644
--- a/libvips/conversion/tilecache.c
+++ b/libvips/conversion/tilecache.c
@@ -935,7 +935,12 @@ vips_line_cache_build( VipsObject *object )
 	vips_get_tile_size( block_cache->in, 
 		&tile_width, &tile_height, &n_lines );
 	block_cache->tile_width = block_cache->in->Xsize;
-	block_cache->max_tiles = 1 + 2 * n_lines / block_cache->tile_height;
+
+	/* Output has two buffers n_lines height, so 2 * n_lines is the maximum
+	 * non-locality from threading. Add another n_lines for conv / reducev
+	 * etc. 
+	 */
+	block_cache->max_tiles = 3 * n_lines / block_cache->tile_height;
 
 	VIPS_DEBUG_MSG( "vips_line_cache_build: n_lines = %d\n", 
 		n_lines );
diff --git a/libvips/iofuncs/threadpool.c b/libvips/iofuncs/threadpool.c
index 76bc4090..a29fb812 100644
--- a/libvips/iofuncs/threadpool.c
+++ b/libvips/iofuncs/threadpool.c
@@ -48,9 +48,9 @@
  */
 
 /* 
-#define VIPS_DEBUG
 #define VIPS_DEBUG_RED
 #define DEBUG_OUT_OF_THREADS
+#define VIPS_DEBUG
  */
 
 #ifdef HAVE_CONFIG_H
@@ -1011,6 +1011,10 @@ vips__threadpool_init( void )
  * Pick a tile size and a buffer height for this image and the current
  * value of vips_concurrency_get(). The buffer height 
  * will always be a multiple of tile_height.
+ *
+ * The buffer height is the height of each buffer we fill in sink disc. Since
+ * we have two buffers, the largest range of input locality is twice the output
+ * buffer size, plus whatever margin we add for things like convolution. 
  */
 void
 vips_get_tile_size( VipsImage *im, 
@@ -1054,9 +1058,10 @@ vips_get_tile_size( VipsImage *im,
 	 * to a multiple of tileheight.
 	 */
 	*n_lines = vips__tile_height * 
-		(1 + nthr / VIPS_MAX( 1, im->Xsize / vips__tile_width )) * 2;
-	*n_lines = VIPS_MAX( *n_lines, vips__fatstrip_height * nthr * 2 );
-	*n_lines = VIPS_MAX( *n_lines, vips__thinstrip_height * nthr * 2 );
+		VIPS_MAX( 1, 
+			nthr / VIPS_MAX( 1, im->Xsize / vips__tile_width ) );
+	*n_lines = VIPS_MAX( *n_lines, vips__fatstrip_height * nthr );
+	*n_lines = VIPS_MAX( *n_lines, vips__thinstrip_height * nthr );
 	*n_lines = VIPS_ROUND_UP( *n_lines, *tile_height );
 
 	/* We make this assumption in several places.