GNOME Bugzilla – Bug 754182
Add support for 64-bit integers to g_atomic_int*
Last modified: 2018-05-24 18:10:47 UTC
The gatomic documentation says: > There is no support for 64bit operations on platforms with 32bit pointers because it is not generally possible to perform these operations atomically. This is incorrect. The underlying gcc API[1] and the Windows API[2] used by g_atomic_int* already support 64-bit integers, and gcc falls back to a slower function with the same guarantees[3]. Hence, the GCC API guarantees atomicity for 64-bit aligned integers on 32-bit platforms as defined by Intel, and the Windows API guarantees the same. The exact same implementation that is currently used for 32-bit integers would also work for 64-bit (long long) integers. 1. “GCC allows any integral scalar or pointer type that is 1, 2, 4 or 8 bytes in length”, https://gcc.gnu.org/onlinedocs/gcc-4.9.2/gcc/_005f_005fsync-Builtins.html#g_t_005f_005fsync-Builtins 2. “Performs an atomic addition operation on the specified LONGLONG values”, https://msdn.microsoft.com/en-us/library/windows/desktop/ms686360%28v=vs.85%29.aspx#interlocked_functions 3. “If a particular operation cannot be implemented on the target processor, a warning is generated and a call an external function is generated.”, https://gcc.gnu.org/onlinedocs/gcc-4.9.2/gcc/_005f_005fsync-Builtins.html#g_t_005f_005fsync-Builtins
(In reply to Nirbheek Chauhan from comment #0) > gcc falls back to a slower function with the same guarantees[3]. To clarify, it only does this on processors that do not support atomic operations on long long integers. 32-bit x86 processors do support atomic operations on long long integers.
Created attachment 310266 [details] [review] gatomic: Add atomic operations for 64-bit integers While creating this patch, I noticed that a lot more of the g_atomic_int API can make use of the C11-style atomic extension of GCC (see bug 730807). There's scope for improved performance with that since various parts of GStreamer use the inc/swap/etc atomic int API. I'll file a new bug for that later.
The problem is that in many cases we won't get actual atomic operations, but as noted in some of the documentation you link, emulated ones. In the case of emulated calls, the calling process simply acquires a mutex before performing the "atomic" operation in question and as long as everyone else in the process does the same, then the operation is effectively atomic. The problem with this is that it doesn't work against other processes (ie: shared memory regions). We're at a stage where I would rather consider removing the emulated calls that we ourselves have so that we always have real atomic operations without mutexes. Why do you need 64bit atomics on 32bit platforms?
(In reply to desrt's bugzilla bot from comment #3) > The problem is that in many cases we won't get actual atomic operations, but > as noted in some of the documentation you link, emulated ones. > > In the case of emulated calls, the calling process simply acquires a mutex > before performing the "atomic" operation in question and as long as everyone > else in the process does the same, then the operation is effectively atomic. > The problem with this is that it doesn't work against other processes (ie: > shared memory regions). > Would you agree that the most relevant architectures on Linux are x86, arm, and mips? If so, then 8-byte atomic int ops are supported for those architectures with the correct combination of gcc and linux kernel. https://gcc.gnu.org/wiki/Atomic#Status Specifically: “ARM supports 8 byte atomics with cmpxchg64 kernel helper with gcc-4.8 + Linux 3.1 (or Linux 2.6.30 w/ armv6k+)” According to the code, on armv6k+ gcc uses ldrexd/strexd. Also, are you sure gcc falls back to emulating atomic ops with mutexes? Because I thought the same about the __sync() builtins, but further reading told me that if gcc cannot generate atomic ops code for an architecture, it generates calls that must be provided by an external library or the build will fail at link time. From the same page: “All other architectures simply generate calls to the appropriate sync function name, and will fail at link time if a library with the required routines is not provided.” Looking at the atomic code in libgcc, this is indeed the case except on ARM where the latest libgcc calls an abort() if the kernel is not new enough. > We're at a stage where I would rather consider removing the emulated calls > that we ourselves have so that we always have real atomic operations without > mutexes. > When I saw the code, it occurred to me that it would be really nice to at least have a compiler warning when atomic int ops were being emulated with pthread mutexes. Otherwise people see bad performance and/or bugs with shm and are left wondering why. From what I can tell, OS X and iOS both support 64-bit atomic ops on 32-bit platforms other than PPC, so that's covered, and x86 supports this in hardware, so Windows API supports this as well. > Why do you need 64bit atomics on 32bit platforms? In GStreamer, we store timestamps as integer nanoseconds, and we're starting to see reports from people about us overflowing the 32-bit integer limit (bug 754180). We'd like to start using 64-bit integers for such uses in GStreamer, but we'll be limited by the lack of 64-bit atomic int op support in GLib.
What should happen with this now? It would be really good to get this merged for the next GLib major release.
Had another use case today where I could really have used this :)
Review of attachment 310266 [details] [review]: I mean, okay... sure... I still don't know why, but also, why not? One comment: please don't bother with the symbol additions in the .c file. IIRC the only reason we do that there for the other functions is for ABI back-compat. We don't need that with new functions which will always evaluate to the inline versions.
Review of attachment 310266 [details] [review]: Sorry: one more thing too. Please mention something like: "64bit atomic operations may be emulated using a mutex on 32-bit platforms, and should not be used on cross-process shared memory in portable applications."
Review of attachment 310266 [details] [review]: ::: glib/gatomic.c @@ +311,3 @@ + * Returns: the value of the integer + * + * Since: 2.46 We’re approaching 2.52 now, so these need to be updated. ::: glib/gatomic.h @@ +55,3 @@ guint val); +GLIB_AVAILABLE_IN_2_46 GLIB_AVAILABLE_IN_2_52 now.
@pwithnall: yeah, I will update those. @desrt: Maybe I should also define the MSVC implementations of 64-bit atomic ops in the header so it gets inlined too? Also, where should I put the gtk-doc comment if I remove this? Actually, looking at the patch in detail again, I realise that the MSVC and MinGW portions are incorrect; we need to use the 64-bit versions of InterlockedAnd() and friends[1][2] because `long` on Windows is 32-bit. They are available everywhere on MSVC (since 2008), but I will have to test on MinGW32/64 to see if the patch works. The configure bits are also missing for checking 64-bit support. Will reattach again within a week once I've sorted this out. 1. https://msdn.microsoft.com/en-us/library/windows/desktop/ms683527.aspx 2. https://msdn.microsoft.com/en-us/library/windows/desktop/ms683516.aspx
Nirbheek, any news on this?
I've been a bit busy, so wasn't able to get to this. I plan to get to this next week, so will update again then.
Ping? :-)
Pingity ping? (-:
Knock knock.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1076.