mbox

[bug#35666,0/2] Build a thread-safe hdf5 library

Message ID 20190510095630.16695-1-ludo@gnu.org
Headers show

Message

Ludovic Courtès May 10, 2019, 9:56 a.m. UTC
Hello!

A colleague of mine noticed that our ‘hdf5’ library wasn’t
thread-safe.  Turns out there’s an option to make it thread-safe (oh!),
it’s turned off by default (oh?!), and when you pass it ‘configure’
invites you to turn off no less than C++, Fortran, and the high-level
interface (d’oh!).

It also tells you that, if you insist, you can go ahead and pass
‘--enable-unsupported’, but you’re on your own.

We found that Debian chose to pass ‘--enable-unsupported’, and indeed
that seems to be saner than providing a variant that does very little,
but does it in a thread-safe way.

Thoughts?

Thanks,
Ludo’.

Ludovic Courtès (2):
  gnu: hdf5: Build a thread-safe library.
  gnu: hdf5: Add dependency on Perl.

 gnu/packages/maths.scm | 15 +++++++++++++--
 1 file changed, 13 insertions(+), 2 deletions(-)

Comments

Ricardo Wurmus May 10, 2019, 11:52 a.m. UTC | #1
Ludovic Courtès <ludo@gnu.org> writes:

> A colleague of mine noticed that our ‘hdf5’ library wasn’t
> thread-safe.  Turns out there’s an option to make it thread-safe (oh!),
> it’s turned off by default (oh?!), and when you pass it ‘configure’
> invites you to turn off no less than C++, Fortran, and the high-level
> interface (d’oh!).

Oh!

> It also tells you that, if you insist, you can go ahead and pass
> ‘--enable-unsupported’, but you’re on your own.
>
> We found that Debian chose to pass ‘--enable-unsupported’, and indeed
> that seems to be saner than providing a variant that does very little,
> but does it in a thread-safe way.

What other effects does “--enable-unsupported” have?  I see that in
Fedora “--enable-threadsafe” was removed in 2008 because it’s
“incompatible with --enable-cxx and --enable-fortran”.

Instead they seem to be building different flavours: one with
--enable-fortran, another with --enable-cxx, yet another with MPI and
--enable-parallel.

Do we have contact to the hdf5 developers to ask what the implications
of “enable-unsupported” are?

--
Ricardo
Ludovic Courtès May 10, 2019, 1:07 p.m. UTC | #2
Hi!

Ricardo Wurmus <rekado@elephly.net> skribis:

> Ludovic Courtès <ludo@gnu.org> writes:

[…]

>> It also tells you that, if you insist, you can go ahead and pass
>> ‘--enable-unsupported’, but you’re on your own.
>>
>> We found that Debian chose to pass ‘--enable-unsupported’, and indeed
>> that seems to be saner than providing a variant that does very little,
>> but does it in a thread-safe way.
>
> What other effects does “--enable-unsupported” have?  I see that in
> Fedora “--enable-threadsafe” was removed in 2008 because it’s
> “incompatible with --enable-cxx and --enable-fortran”.

“--enable-unsupported” allows you to force a build that combines C++,
Fortran, and thread-safety.  If you don’t pass that flag, you have to
choose between thread-safety and C++/Fortran¹.  A tough choice!

> Instead they seem to be building different flavours: one with
> --enable-fortran, another with --enable-cxx, yet another with MPI and
> --enable-parallel.

Problem is, my colleagues have code that expects both C++ and
thread-safety (as crazy as it might seem).  They were using the Debian
package until now and hadn’t realized about this.

> Do we have contact to the hdf5 developers to ask what the implications
> of “enable-unsupported” are?

I think it’s a warranty-void kind of flag: by passing it, the user
asserts they understand they’re using a configuration not “officially
supported” by the HDF Group, meaning that if it’s buggy, we’re on our
own.

Thoughts?

Ludo’.

¹ You would think it’s an April fool’s day prank, but it’s not!  We’re
  in May, at least in my timezone.
Eric Bavier May 10, 2019, 3:09 p.m. UTC | #3
I think this should be fine, though I've not heard of anyone who has relied on this feature.  The "unsupported" part here is that the posix lock used for thread-safety is not hoisted into the higher-level API calls.  So if your colleague is using the C++ interface and expecting thread-safety, they are out of luck.  So the disclaimer is that only the low-level C interface gains thread-safety, and the rest are no better.

Eric Bavier, Scientific Libraries, Cray Inc.
Paul Garlick May 10, 2019, 3:27 p.m. UTC | #4
Hi Ludo and Ricardo,

> Thoughts?
> 

I notice in the Debian rules file [1] that the "--enable-threadsafe"
flag is listed as  one of the SERIAL_FLAGS.  In the Guix case the
package hdf5-parallel-openmpi inherits its configure flags from the
hdf5 (serial) package.

Will we also have to explicitly delete the "--enable-threadsafe" flag
from the hdf5-parallel-openmpi definition?

Best regards,

Paul.

[1] https://sources.debian.org/src/hdf5/1.10.4+repack-10/debian/rules/
Ludovic Courtès May 14, 2019, 7:28 a.m. UTC | #5
Hi Eric,

Eric Bavier <bavier@cray.com> skribis:

> I think this should be fine, though I've not heard of anyone who has
> relied on this feature.  The "unsupported" part here is that the posix
> lock used for thread-safety is not hoisted into the higher-level API
> calls.  So if your colleague is using the C++ interface and expecting
> thread-safety, they are out of luck.  So the disclaimer is that only
> the low-level C interface gains thread-safety, and the rest are no
> better.

I’m not sure I understand.  Do you mean that, just because you use the
C++ API instead of the C API, the library is not thread-safe?

They do see crashes vanish when using the library compiled with
‘--enable-threadsafe’, and reliably so.

Thanks,
Ludo’.
Ludovic Courtès May 14, 2019, 10:21 a.m. UTC | #6
Hi Paul,

Paul Garlick <pgarlick@tourbillion-technology.com> skribis:

> I notice in the Debian rules file [1] that the "--enable-threadsafe"
> flag is listed as  one of the SERIAL_FLAGS.  In the Guix case the
> package hdf5-parallel-openmpi inherits its configure flags from the
> hdf5 (serial) package.
>
> Will we also have to explicitly delete the "--enable-threadsafe" flag
> from the hdf5-parallel-openmpi definition?

Good point, I think so.

I did that and pushed it as 549d15712fdc1f58ce0dd11117eb79535ec19f2c.

Let me know if anything is amiss!

Ludo’.
Ricardo Wurmus May 14, 2019, 12:02 p.m. UTC | #7
Ludovic Courtès <ludovic.courtes@inria.fr> writes:

>> Do we have contact to the hdf5 developers to ask what the implications
>> of “enable-unsupported” are?
>
> I think it’s a warranty-void kind of flag: by passing it, the user
> asserts they understand they’re using a configuration not “officially
> supported” by the HDF Group, meaning that if it’s buggy, we’re on our
> own.

I don’t object to adding the option.  It sounds like you confirmed that
it fixes serious problems in practise, so I think it’s a good idea to
enable it.

--
Ricardo
Eric Bavier May 14, 2019, 2:40 p.m. UTC | #8
> I’m not sure I understand.  Do you mean that, just because you use the
> C++ API instead of the C API, the library is not thread-safe?

The thread-safety of the C++ interface itself is not guaranteed/"supported".

> They do see crashes vanish when using the library compiled with
> ‘--enable-threadsafe’, and reliably so.

Great.  I'm not familiar enough with the C++ interface code to say which areas might cause problems in a threaded context.  It's likely that whatever problems they were seeing before was not at the interface layer but deeper.

Eric Bavier, Scientific Libraries, Cray Inc.