git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
* Safe to use stdatomic.h?
@ 2017-03-20 20:18 Ben Peart
  2017-03-20 20:27 ` Jeff King
  2017-03-20 23:22 ` brian m. carlson
  0 siblings, 2 replies; 4+ messages in thread
From: Ben Peart @ 2017-03-20 20:18 UTC (permalink / raw)
  To: git; +Cc: Jeff Hostetler \(jeffhost, Ben Peart

My college Jeff is working on a patch series to further parallelize the
loading of the index.  As part of that patch, it would be nice to use the
atomic_fetch_add function as that would be more efficient than creating a
mutex simply to protect a variable so that it can be incremented.  I haven't
seen any use of atomics yet in Git, nor anything that includes
<stdatomic.h>.

GCC has supported them since 4.9 and Clang has supported them by default
since 3.3.  Are there any compilers currently in use by Git that don't
support these C11 functions?

Thanks,

Ben



^ permalink raw reply	[flat|threaded] 4+ messages in thread

* Re: Safe to use stdatomic.h?
  2017-03-20 20:18 Safe to use stdatomic.h? Ben Peart
@ 2017-03-20 20:27 ` Jeff King
  2017-03-20 23:22 ` brian m. carlson
  1 sibling, 0 replies; 4+ messages in thread
From: Jeff King @ 2017-03-20 20:27 UTC (permalink / raw)
  To: Ben Peart; +Cc: git, Jeff Hostetler (jeffhost, Ben Peart

On Mon, Mar 20, 2017 at 04:18:20PM -0400, Ben Peart wrote:

> My college Jeff is working on a patch series to further parallelize the
> loading of the index.  As part of that patch, it would be nice to use the
> atomic_fetch_add function as that would be more efficient than creating a
> mutex simply to protect a variable so that it can be incremented.  I haven't
> seen any use of atomics yet in Git, nor anything that includes
> <stdatomic.h>.
> 
> GCC has supported them since 4.9 and Clang has supported them by default
> since 3.3.  Are there any compilers currently in use by Git that don't
> support these C11 functions?

Yes, there definitely are older compilers in use. However, if you can
abstract the operation you want to perform into its own function, it's
perfectly fine to #ifdef it to use the atomics when available and fall
back to a mutex otherwise.

-Peff

^ permalink raw reply	[flat|threaded] 4+ messages in thread

* Re: Safe to use stdatomic.h?
  2017-03-20 20:18 Safe to use stdatomic.h? Ben Peart
  2017-03-20 20:27 ` Jeff King
@ 2017-03-20 23:22 ` brian m. carlson
  2017-03-21  1:59   ` Eric Wong
  1 sibling, 1 reply; 4+ messages in thread
From: brian m. carlson @ 2017-03-20 23:22 UTC (permalink / raw)
  To: Ben Peart; +Cc: git, Jeff Hostetler (jeffhost, Ben Peart

[-- Attachment #1: Type: text/plain, Size: 1187 bytes --]

On Mon, Mar 20, 2017 at 04:18:20PM -0400, Ben Peart wrote:
> My college Jeff is working on a patch series to further parallelize the
> loading of the index.  As part of that patch, it would be nice to use the
> atomic_fetch_add function as that would be more efficient than creating a
> mutex simply to protect a variable so that it can be incremented.  I haven't
> seen any use of atomics yet in Git, nor anything that includes
> <stdatomic.h>.
> 
> GCC has supported them since 4.9 and Clang has supported them by default
> since 3.3.  Are there any compilers currently in use by Git that don't
> support these C11 functions?

At work, we're compiling for CentOS 6 and 7.  CentOS 7 only has GCC 4.8,
and CentOS 6 has something much older.  This is code we ship to
customers, so we can't rely on them having devtoolset installed for
newer GCC.

I could support the argument for ditching RHEL/CentOS 5 support, but I
expect other people might disagree.  After all, we're still targeting
C89.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 868 bytes --]

^ permalink raw reply	[flat|threaded] 4+ messages in thread

* Re: Safe to use stdatomic.h?
  2017-03-20 23:22 ` brian m. carlson
@ 2017-03-21  1:59   ` Eric Wong
  0 siblings, 0 replies; 4+ messages in thread
From: Eric Wong @ 2017-03-21  1:59 UTC (permalink / raw)
  To: brian m. carlson, Ben Peart, git, Jeff Hostetler, Ben Peart

"brian m. carlson" <sandals@crustytoothpaste.net> wrote:
> I could support the argument for ditching RHEL/CentOS 5 support, but I
> expect other people might disagree.  After all, we're still targeting
> C89.

Yeah, I still use and support CentOS 5 in some places (but maybe
not git, still using ancient versions there, too).

Anyways, I'm still relying on the traditional __sync_* builtins
from in earlier gcc 4.x releases in some code GPL-3.0 code I
maintain for older systems:

	https://gcc.gnu.org/onlinedocs/gcc/_005f_005fsync-Builtins.html

Since git is GPL-2.0, it is license-compatible with all the
atomic macros in the Linux kernel, as well as the kernel-derived
userspace atomics (uatomic) found in liburcu <http://liburcu.org/>

^ permalink raw reply	[flat|threaded] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-03-20 20:18 Safe to use stdatomic.h? Ben Peart
2017-03-20 20:27 ` Jeff King
2017-03-20 23:22 ` brian m. carlson
2017-03-21  1:59   ` Eric Wong

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox