split up rw/_mutex.h

• Key: STDCXX-563
• Type: Improvement
• Status: Resolved
• Resolution: Fixed
• Priority: Minor
• Assignee: Farid Zaripov
• Reporter: Martin Sebor
• Component/s: Build and Installation
• Affects Version/s: 4.2.0
• Fix Version/s: 4.2.2

Description

The internal header <rw/_mutex.h> has become too big and hard to maintain. It would be an improvement to split it up into multiple headers, one for each supported implementation of threads, along the lines of what was done with the <rw/_config.h> header in http://svn.apache.org/viewvc?view=rev&revision=382600. This is too big to do for 4.2 but simple enough that it could go in 4.2.1.

Comments

[Martin Sebor]

Now that 4.2.0 is released, set Affects Version(s) accordingly.

[Martin Sebor]

A few things to consider:

1. The long term goal should be to eliminate the dependency of our headers on any particular implementation of the thread library and abstract everything under a common (stdcxx-only) interface.

2. Another long term goal should be to eliminate the binary incompatibility between reentrant (thread-safe) and ordinary builds.

3. In the meantime, does it make sense to introduce platform-specific subdirectories into the include directory (along the same lines as what we have under src/) or should we follow the _config.h approach taken in rev 382600 (i.e., append a suffix to the basename of the _mutex.h header)?

[Martin Sebor]

See a proposed patch here: http://www.nabble.com/-PATCH--STDCXX-563-to15186746.html

[Martin Sebor]

Added an initial guesstimate and set Severity to Cosmetic.

[Farid Zaripov]

The patch is attached.

[Martin Sebor]

Here are some observations and suggestions regarding the patch:

1. Underscores separating components of file names should be replaced with dashes for consistency with the rw/_config-*.h headers.
2. There's a typo in the name of _atomic_aplha.h. I suspect the name should be changed to _atomic-deccxx.h since the primitives seem specific to the compiler, not to the hardware architecture.
3. What is _atomic_generic.h for and shouldn't it be merged with _atomic.h?
4. What compilers is _atomic_ia64_x64.h used by? If all of them on IA64 as well as x86_64 (in LLP64), maybe it should be called _atomic-x64.h. I see a lof of #ifdefs for MSVC. Would it make sense to split it up into _atomic-msvc.h and whatever else?
5. I believe _atomic_mips.h is specific to the MIPSpro compiler and couldn't be used with gcc on the MIPS architecture. It should be renamed to _atomic_mipspro.h
6. I'm not quite sure what to do with _atomic_mutex.h. Ideally, we would have atomic operations everywhere. If there is a platform where we (sometimes) need to use the mutex version (I think you mentioned PA-RISC) I guess we need to keep it but it doesn't make me very happy...
7. If _atomic_powerpc.h is specific to IBM XLC++ (and can't be used by gcc) it should be renamed to _atomic-xlc.h.
8. One of _atomic-x86.h and the src/i86 directory should be renamed for consistency. It seems that the commonly used abbreviation used for the Intel 8086-derived processors (e.g., 80386, 80486) is x86 – see the Wikipedia article.
9. Would _mutex-win32.h be a better name than _mutex-windows.h?

Finally, I wonder if instead of adding suffixes to these files and worry about being consistent every time we add a new one it would make sense to add platform-specific directories under include/rw/ instead and move the corresponding files (as well as the rw/_config-*.h headers) there. Thoughts?

[Martin Sebor]

Deferred until 4.2.2.

This would be nice to have but it's not essential. Lowered Priority to Minor.

[Farid Zaripov]

New patch attached.

ChangeLog:

	STDCXX-563
	* include/rw/_atomic-deccxx.h: New header file with definitions of inline
	functions for atomic operations on ALPHA platform.
	* include/rw/_atomic-mipspro.h: New header file with definitions of inline functions
	for atomic operations on MIPS platform.
	* include/rw/_atomic-mutex.h: New header file with definitions of inline functions
	for atomic operations with using mutex object.
	* include/rw/_atomic-parisc.h: New header file with definitions of inline functions
	for atomic operations on PA RISC platform.
	* include/rw/_atomic-sparc.h: New header file with definitions of inline functions
	for atomic operations on SPARC platform.
	* include/rw/_atomic-x64.h: New header file with definitions of inline
	functions for atomic operations on Intel IA64 and X64 platforms.
	* include/rw/_atomic-x86.h: New header file with definitions of inline functions for
	atomic operations on Intel X86 platform.
	* include/rw/_atomic-xlc.h: New header file with definitions of inline functions
	for atomic operations on POWERPC platform.
	* include/rw/_atomic.h: New header file with definitions of inline functions
	for atomic operations.
	* include/rw/_mutex-dce.h: New header file with definitions of classes for thread
	safety using DCE threads.
	* include/rw/_mutex-os2.h: New header file with definitions of classes for thread
	safety using OS2 threads.
	* include/rw/_mutex-pthread.h: New header file with definitions of classes for thread
	safety using POSIX threads.
	* include/rw/_mutex-solaris.h: New header file with definitions of classes for thread
	safety using Solaris threads.
	* include/rw/_mutex-win32.h: New header file with definitions of classes for thread
	safety using Windows threads.
	* include/rw/_mutex.h: Split content of the file to the set of platform specific and OS
	specific headers above.
	(__rw_get_static_mutex) [!_RWSTD_NO_ATOMIC_OPS && !_PA_RISC2_0]: Use
	_RWSTD_ATOMIC_PREINCREMENT() on all platforms where atomic increment is
	available instead of using _InterlockedIncrement() only on Windows.

[Farid Zaripov]

Resolved thus: http://svn.apache.org/viewvc?view=rev&revision=696295

[Farid Zaripov]

Merged into 4.3.x branch and trunk thus:
http://svn.apache.org/viewvc?view=rev&revision=704152
http://svn.apache.org/viewvc?view=rev&revision=704153