starpu_data_dup_ro: optimize when src_handle already has a read-only duplicate

src/core/dependencies/data_concurrency.c

@@ -142,8 +142,6 @@ static unsigned _starpu_attempt_to_submit_data_request(unsigned request_from_cod
 		_starpu_spin_lock(&handle->header_lock);
 	}
 
-	STARPU_ASSERT_MSG(!(handle->readonly && (mode & STARPU_W)), "Read-only handles can not be written to");
-
 	/* If we have a request that is not used for the reduction, and that a
 	 * reduction is pending, we put it at the end of normal list, and we
 	 * use the reduction_req_list instead */

src/core/dependencies/implicit_data_deps.c

@@ -227,7 +227,12 @@ struct starpu_task *_starpu_detect_implicit_data_deps_with_handle(struct starpu_
 
 		if (mode & STARPU_W || mode == STARPU_REDUX)
 		{
+
+			STARPU_ASSERT_MSG(!handle->readonly, "Read-only handles can not be written to");
+
 			handle->initialized = 1;
+			/* We will change our value, disconnect from our readonly duplicates */
+			handle->readonly_dup = NULL;
 			if (write_hook)
 				write_hook(handle);
 		}

src/datawizard/coherency.h

@@ -194,6 +194,9 @@ struct _starpu_data_state
 	/** for a readonly handle, the number of times that we have returned again the
 	    same handle and thus the number of times we have to ignore unregistration requests */
 	unsigned aliases;
+	/** for a non-readonly handle, a readonly-only duplicate, that we can
+	    return from starpu_data_dup_ro */
+	starpu_data_handle_t readonly_dup;
 
 	/** in some case, the application may explicitly tell StarPU that a
  	 * piece of data is not likely to be used soon again */

src/datawizard/interfaces/data_interface.c

@@ -808,7 +808,7 @@ static void _starpu_data_unregister(starpu_data_handle_t handle, unsigned cohere
 		STARPU_ASSERT_MSG(_starpu_worker_may_perform_blocking_calls(), "starpu_data_unregister must not be called from a task or callback, perhaps you can use starpu_data_unregister_submit instead");
 
 		/* If sequential consistency is enabled, wait until data is available */
-		_starpu_data_wait_until_available(handle, STARPU_RW, "starpu_data_unregister");
+		_starpu_data_wait_until_available(handle, handle->readonly?STARPU_R:STARPU_RW, "starpu_data_unregister");
 	}
 
 	if (coherent && !nowait)

src/util/starpu_data_cpy.c

@@ -180,6 +180,17 @@ int starpu_data_dup_ro(starpu_data_handle_t *dst_handle, starpu_data_handle_t sr
 			int asynchronous, void (*callback_func)(void*), void *callback_arg)
 {
 	_starpu_spin_lock(&src_handle->header_lock);
+	if (src_handle->readonly_dup) {
+		/* Already a ro duplicate, just return it with one more ref */
+		*dst_handle = src_handle->readonly_dup;
+		_starpu_spin_unlock(&src_handle->header_lock);
+		_starpu_spin_lock(&(*dst_handle)->header_lock);
+		(*dst_handle)->aliases++;
+		_starpu_spin_unlock(&(*dst_handle)->header_lock);
+		if (callback_func)
+			callback_func(callback_arg);
+		return 0;
+	}
 	if (src_handle->readonly) {
 		src_handle->aliases++;
 		_starpu_spin_unlock(&src_handle->header_lock);
@@ -190,9 +201,13 @@ int starpu_data_dup_ro(starpu_data_handle_t *dst_handle, starpu_data_handle_t sr
 	}
 	_starpu_spin_unlock(&src_handle->header_lock);
 
-	/* TODO: optimize when src_handle already has a read-only duplicate */
 	starpu_data_register_same(dst_handle, src_handle);
 	_starpu_data_cpy(*dst_handle, src_handle, asynchronous, callback_func, callback_arg, 0, NULL);
 	(*dst_handle)->readonly = 1;
+
+	_starpu_spin_lock(&src_handle->header_lock);
+	src_handle->readonly_dup = (*dst_handle);
+	_starpu_spin_unlock(&src_handle->header_lock);
+
 	return 0;
 }

tests/Makefile.am

@@ -793,6 +793,14 @@ fortran90_init_01_SOURCES =	\
 	fortran90/init_01.f90
 endif
 
+helper_starpu_data_dup_ro_SOURCES =		\
+	helper/starpu_data_dup_ro.c		\
+	main/increment_codelet.c
+if STARPU_USE_CUDA
+helper_starpu_data_dup_ro_SOURCES +=		\
+	main/increment.cu
+endif
+
 ###################
 # Block interface #
 ###################

tests/helper/starpu_data_dup_ro.c

@@ -15,6 +15,7 @@
  */
 
 #include <starpu.h>
+#include "../main/increment_codelet.h"
 #include "../helper.h"
 
 /*
@@ -24,8 +25,8 @@
 int main(int argc, char **argv)
 {
 	int ret;
-	int var1, *var2, *var3, *var4;
-	starpu_data_handle_t var1_handle, var2_handle, var3_handle, var4_handle;
+	unsigned var1, *var;
+	starpu_data_handle_t var1_handle, var2_handle, var3_handle, var4_handle, var5_handle;
 
 	ret = starpu_initialize(NULL, &argc, &argv);
 	if (ret == -ENODEV) return STARPU_TEST_SKIPPED;
@@ -42,47 +43,64 @@ int main(int argc, char **argv)
 	/* Make a second duplicate of the original data */
 	ret = starpu_data_dup_ro(&var3_handle, var1_handle, 1, NULL, NULL);
 	STARPU_CHECK_RETURN_VALUE(ret, "starpu_data_dup_ro");
+	STARPU_ASSERT(var3_handle == var2_handle);
 
 	/* Make a duplicate of a duplicate */
 	ret = starpu_data_dup_ro(&var4_handle, var2_handle, 1, NULL, NULL);
 	STARPU_CHECK_RETURN_VALUE(ret, "starpu_data_dup_ro");
-
 	STARPU_ASSERT(var4_handle == var2_handle);
 
+	starpu_task_insert(&increment_codelet, STARPU_RW, var1_handle, 0);
+
+	/* Make a duplicate of the new value */
+	ret = starpu_data_dup_ro(&var5_handle, var1_handle, 1, NULL, NULL);
+	STARPU_CHECK_RETURN_VALUE(ret, "starpu_data_dup_ro");
+
 	starpu_data_acquire(var2_handle, STARPU_R);
-	var2 = starpu_data_get_local_ptr(var2_handle);
+	var = starpu_data_get_local_ptr(var2_handle);
 	ret = EXIT_SUCCESS;
-	if (*var2 != var1)
+	if (*var != 42)
 	{
-	     FPRINTF(stderr, "var2 is %d but it should be %d\n", *var2, var1);
+	     FPRINTF(stderr, "var2 is %d but it should be %d\n", *var, 42);
 	     ret = EXIT_FAILURE;
 	}
 	starpu_data_release(var2_handle);
 
 	starpu_data_acquire(var3_handle, STARPU_R);
-	var3 = starpu_data_get_local_ptr(var3_handle);
+	var = starpu_data_get_local_ptr(var3_handle);
 	ret = EXIT_SUCCESS;
-	if (*var3 != var1)
+	if (*var != 42)
 	{
-	     FPRINTF(stderr, "var3 is %d but it should be %d\n", *var3, var1);
+	     FPRINTF(stderr, "var3 is %d but it should be %d\n", *var, 42);
 	     ret = EXIT_FAILURE;
 	}
 	starpu_data_release(var3_handle);
 
 	starpu_data_acquire(var4_handle, STARPU_R);
-	var4 = starpu_data_get_local_ptr(var4_handle);
+	var = starpu_data_get_local_ptr(var4_handle);
 	ret = EXIT_SUCCESS;
-	if (*var4 != var1)
+	if (*var != 42)
 	{
-	     FPRINTF(stderr, "var4 is %d but it should be %d\n", *var4, var1);
+	     FPRINTF(stderr, "var4 is %d but it should be %d\n", *var, 42);
 	     ret = EXIT_FAILURE;
 	}
 	starpu_data_release(var4_handle);
 
+	starpu_data_acquire(var5_handle, STARPU_R);
+	var = starpu_data_get_local_ptr(var5_handle);
+	ret = EXIT_SUCCESS;
+	if (*var != 43)
+	{
+	     FPRINTF(stderr, "var5 is %d but it should be %d\n", *var, 43);
+	     ret = EXIT_FAILURE;
+	}
+	starpu_data_release(var5_handle);
+
 	starpu_data_unregister(var1_handle);
 	starpu_data_unregister(var2_handle);
 	starpu_data_unregister(var3_handle);
 	starpu_data_unregister(var4_handle);
+	starpu_data_unregister(var5_handle);
 	starpu_shutdown();
 
 	STARPU_RETURN(ret);