[m-rev.] for review: version array thread_safe attributes

Peter Wang novalazy at gmail.com
Fri Apr 13 16:17:48 AEST 2012


It turns out that the C implementation of version arrays wasn't ever
thread-unsafe due to the global lock.  The Java (and C#) backend don't
implement a global lock and that's why we saw thread-safety issues, and
added locking to version arrays.  As a result, the C implementation is
extremely thread-safe!  Anyway, per-array locks are better than relying
on the global lock so it all worked out, eventually.
--

Branches: main, 11.07

library/version_array.m:
	Set `thread_safe' attributes on version array predicates,
	with justification.

	Conform to renaming of `new' to `init' previously.
	The Java and C# foreign_procs were not renamed.

	Reword a bit of documentation.

diff --git a/library/version_array.m b/library/version_array.m
index 91f2bf1..298a364 100644
--- a/library/version_array.m
+++ b/library/version_array.m
@@ -73,21 +73,21 @@
 
     % Same as empty/0 except the resulting version_array is not thread safe.
     %
-    % That is your program can segfault if you attempt to concurrently access
-    % or update the array from different threads, or any two arrays produced
-    % from operations on the same original array.  However this version is much
-    % quicker if you guarantee that you never concurrently access the version
-    % array.
+    % That is your program can crash or behave strangely if you attempt to
+    % concurrently access or update the array from different threads, or any
+    % two arrays produced from operations on the same original array.
+    % However this version is much quicker if you guarantee that you never
+    % concurrently access the version array.
     %
 :- func unsafe_empty = version_array(T).
 
     % Same as new(N, X) except the resulting version_array is not thread safe.
     %
-    % That is your program can segfault if you attempt to concurrently access
-    % or update the array from different threads, or any two arrays produced
-    % from operations on the same original array.  However this version is much
-    % quicker if you guarantee that you never concurrently access the version
-    % array.
+    % That is your program can crash or behave strangely if you attempt to
+    % concurrently access or update the array from different threads, or any
+    % two arrays produced from operations on the same original array.
+    % However this version is much quicker if you guarantee that you never
+    % concurrently access the version array.
     %
 :- func unsafe_new(int, T) = version_array(T).
 
@@ -423,8 +423,13 @@ unsafe_rewind(VA, unsafe_rewind(VA)).
 %-----------------------------------------------------------------------------%
 % Sordid stuff below this point...
 %
-% Note: this code is not thread safe, hence the absence of `thread_safe'
-% attributes!
+% The `thread_safe' attributes are justified:
+% - creating new version arrays is thread-safe
+% - thread-safe version arrays are protected by their own locks so do not need
+%   the global lock
+% - the whole point of providing non-thread-safe version arrays is to avoid
+%   locking when the user "knows", and supposedly guarantees, that it is safe
+%   to do so.
 
 :- pragma foreign_type("C", version_array(T), "struct ML_va *")
     where
@@ -506,7 +511,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("C",
     version_array.empty = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     MR_Word array;
@@ -529,7 +534,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("C#",
     version_array.empty = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     VA = new version_array.ML_sva(version_array.ML_uva.empty());
@@ -537,7 +542,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("Java",
     version_array.empty = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     VA = new ML_sva(ML_uva.empty());
@@ -545,7 +550,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("C",
     version_array.unsafe_empty = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     MR_Word array;
@@ -567,7 +572,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("C#",
     version_array.unsafe_empty = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     VA = version_array.ML_uva.empty();
@@ -575,7 +580,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("Java",
     version_array.unsafe_empty = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     VA = ML_uva.empty();
@@ -583,7 +588,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("C",
     version_array.init(N::in, X::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness, may_not_duplicate],
 "
     MR_Integer  i;
@@ -610,16 +615,16 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 ").
 
 :- pragma foreign_proc("C#",
-    version_array.new(N::in, X::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    version_array.init(N::in, X::in) = (VA::out),
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness, may_not_duplicate],
 "
     VA = new version_array.ML_sva(version_array.ML_uva.init(N, X));
 ").
 
 :- pragma foreign_proc("Java",
-    version_array.new(N::in, X::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    version_array.init(N::in, X::in) = (VA::out),
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness, may_not_duplicate],
 "
     VA = new ML_sva(ML_uva.init(N, X));
@@ -627,7 +632,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("C",
     version_array.unsafe_new(N::in, X::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness, may_not_duplicate],
 "
     MR_Integer  i;
@@ -654,7 +659,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("C#",
     version_array.unsafe_new(N::in, X::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness, may_not_duplicate],
 "
     VA = version_array.ML_uva.init(N, X);
@@ -662,7 +667,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("Java",
     version_array.unsafe_new(N::in, X::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness, may_not_duplicate],
 "
     VA = ML_uva.init(N, X);
@@ -670,7 +675,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("C",
     resize(VA0::in, N::in, X::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     VA = ML_va_resize_dolock(VA0, N, X, MR_ALLOC_ID);
@@ -678,7 +683,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("C#",
     resize(VA0::in, N::in, X::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness, may_not_duplicate],
 "
     VA = VA0.resize(N, X);
@@ -686,7 +691,7 @@ cmp_version_array_2(I, Size, VAa, VAb, R) :-
 
 :- pragma foreign_proc("Java",
     resize(VA0::in, N::in, X::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness, may_not_duplicate],
 "
     VA = VA0.resize(N, X);
@@ -696,7 +701,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("C",
     size(VA::in) = (N::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     N = ML_va_size_dolock(VA);
@@ -704,7 +709,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("C#",
     size(VA::in) = (N::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     N = VA.size();
@@ -712,7 +717,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("Java",
     size(VA::in) = (N::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     N = VA.size();
@@ -722,7 +727,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("C",
     get_if_in_range(VA::in, I::in, X::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     SUCCESS_INDICATOR = ML_va_get_dolock(VA, I, &X);
@@ -730,7 +735,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("C#",
     get_if_in_range(VA::in, I::in, X::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     try {
@@ -744,7 +749,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("Java",
     get_if_in_range(VA::in, I::in, X::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     try {
@@ -761,7 +766,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("C",
     set_if_in_range(VA0::in, I::in, X::in, VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     SUCCESS_INDICATOR = ML_va_set_dolock(VA0, I, X, &VA, MR_ALLOC_ID);
@@ -769,7 +774,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("C#",
     set_if_in_range(VA0::in, I::in, X::in, VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     try {
@@ -783,7 +788,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("Java",
     set_if_in_range(VA0::in, I::in, X::in, VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     try {
@@ -797,7 +802,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("C",
     unsafe_rewind(VA0::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     VA = ML_va_rewind_dolock(VA0);
@@ -805,7 +810,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("C#",
     unsafe_rewind(VA0::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     VA = VA0.rewind();
@@ -813,7 +818,7 @@ resize(N, X, VA, resize(VA, N, X)).
 
 :- pragma foreign_proc("Java",
     unsafe_rewind(VA0::in) = (VA::out),
-    [will_not_call_mercury, promise_pure, will_not_modify_trail,
+    [will_not_call_mercury, promise_pure, thread_safe, will_not_modify_trail,
         does_not_affect_liveness],
 "
     VA = VA0.rewind();

--------------------------------------------------------------------------
mercury-reviews mailing list
Post messages to:       mercury-reviews at csse.unimelb.edu.au
Administrative Queries: owner-mercury-reviews at csse.unimelb.edu.au
Subscriptions:          mercury-reviews-request at csse.unimelb.edu.au
--------------------------------------------------------------------------



More information about the reviews mailing list