revise cache sizing

jcupitt · jcupitt · commit e01a90f7cfef · 2017-04-13T16:24:51.000+01:00
we had output buffers too large, input caches too small see https://github.com/jcupitt/libvips/issues/639
diff --git 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
@@ -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
@@ -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
@@ -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.

Original file line number	Diff line number	Diff line change
`@@ -1,5 +1,3 @@`
`1`		`-- add analytics tags to docs output`
`2`		`-`
`3`	`1`	`- not sure about utf8 error messages on win`
`4`	`2`
`5`	`3`	`- strange:`