Am 26.06.25 um 10:15 schrieb Andre Vehreschild:
Hi Thomas,
I have a few questions.
First, I see that your patch series does not use gfortran's descriptors
for accessing coarrays via shared memory, as the original work by
Nicolas did. Can you comment on that?
The ABI for invoking coarray functionality is sufficient for doing the job.
Modifying the compiler to access coarrays directly, i.e., having implementation
detail on a certain library in the compiler did not appeal to me. Furthermore
has the new library in conjunction with the other library available the
potential to get to a stable and maintained ABI. Having another ABI in the
compiler would have lead to two badly maintained ones (in my opinion). I
therefore decided to just have one ABI and figured that all that is needed can
be done in a library. This also allows link-time polymorphism. And last but not
least, there is a major rework on the array descriptor going on and that would
have had the potential to conflict with my work.
We are very probably not going to get performance out of it that is
comparable with the original design; I would be quite surprised if it
was appreciably better than using shared memory MPI, and in that
case I don't see an advantage of your patch over a better wrapper.
We had a fairly large factor of performance gain versus everything else
with the old, process-based version.
Do you have benchmarks for these three cases?
Second, how did you ensure that the library is free from race
conditions?
The library itself uses mutexes where needed. Additionally are there no
segments where two locks are held at the same time (when I remember correctly).
The question is not if there are mutexes, the question is if they are
used in such a way that there are not race conditions. I believe there
were still some race conditions in the library, did you find and fix
them? Did you do a thorough test, running many parallel images
in such a way that the different parts of the library are stressed?
Do you have the source code for your tests somewhere?
When you mean races on coarray's data: It is not. That is the user's
responsibility like it is with the other coarray implementation.
I've worked on that implementation myself, I think you can assume
that I know about SYNC :-)
[...]
Given the current state of affairs, the patch is not OK.
Best regards
Thomas
