On 28. 2. 25 13:45, Branko Čibej wrote:
On 26. 2. 25 15:20, Stefan Sperling wrote:
On Wed, Feb 26, 2025 at 02:54:43PM +0100, Daniel Sahlberg wrote:
Should have researched a bit more before replying. There was already some
code to do exactly this, but it was removed in r1722879:
[[[
stefan2 2016-01-04 15:17:04
Stop using pointer arithmetics to check for proper alignment because
that is not portable.
As a result, platforms that don't allow unaligned data access will
suffer a small additional performance hit.
* subversion/libsvn_subr/eol.c
(svn_eol__find_eol_start): No longer attempt aligned chunky processing
when unaligned access is not supported.
* subversion/libsvn_subr/utf_validate.c
(first_non_fsm_start_char): Same.
]]]
Don't quite understand why but there is probably a reasonable explanation.
It took us a few rounds of testing to keep things working across
all platforms when these optimizations went in.
As far as I recall, there was run-time fallout on the OpenBSD/sparc64
buildbot (which no longer exists; I don't have sparc64 hardware anymore).
Unaligned access will always cause hardware traps on that platform.
The above change was probably related to that.
With x86 platform behaviour changing all the testing we did back then
is being invalidated. The solution with the least amount of effort on
our part would be using the simple char * loop implementation everywhere.
The maintenance effort required to keep the unaligned-access code path
working and safe on all (modern) platforms would likely exceed our
current development capabilities. Though of course, if someone wants
to do that work, I wouldn't be opposed.
Actually, x86 behaviour hasn't changed wrt alignment for "normal"
memory access in decades. What changed was the addition of vector
instructions, which explicitly disallow unaligned access, and compiler
support for them -- in the reported case it's the -mavx that does it,
while I suspect clang just adds something similar by default at high
enough optimization levels.
At the time, IIRC stefan² tested performance on some (then-)modern x86
CPUs and came away with the result that it's faster to allow the CPU
to handle unaligned access ... or it would be. These kinds of
optimizations always smelled slightly off to me, e.g., why not allow
compiler intrinsics to do their magic instead?
For my €.02, I'd just remove this macro and the micro-optimizations
that go with it and, wherever possible, use standard library functions
instead of the handcrafted ones.
Or, since the macro is public -- deprecate it and stop using it in our
code. The workaround for the reported bug would be adding
-DSVN_UNALIGNED_ACCESS_IS_OK=0 to the CFLAGS.
This is probably the minimal patch to disable this tweak. There's a
bunch of code that needs to be replaced or removed, though.
[[[
Index: notes/knobs
===================================================================
--- notes/knobs (revision 1924097)
+++ notes/knobs (working copy)
@@ -38,7 +38,7 @@ SUFFIX_LINES_TO_KEEP
SVN_FS_FS_DEFAULT_MAX_FILES_PER_DIR
SVN_FS_FS_MAX_LINEAR_DELTIFICATION
SVN_FS_FS_MAX_DELTIFICATION_WALK
-SVN_UNALIGNED_ACCESS_IS_OK
+SVN_UNALIGNED_ACCESS_IS_OK (deprecated)
2.2 Features
@@ -164,12 +164,14 @@ SVN_I_LIKE_LATENCY_SO_IGNORE_HTTPV2
3.8 SVN_UNALIGNED_ACCESS_IS_OK
+ DEPRECATED, unused. The rest of this entry is historical.
+
Scope: (global)
Purpose: enable data accesss optimizations.
If your target CPU supports unaligned memory access without
significant performance penalties, you should enable this
optimization as it allows for processing 4 or 8 bytes at
- once instead of just one byte at a time.
+ once instead of just one byte at a time.
Range: 0 1
Default: platform dependant (see svn_types.h)
Suggested: 0
Index: subversion/include/svn_types.h
===================================================================
--- subversion/include/svn_types.h (revision 1924097)
+++ subversion/include/svn_types.h (working copy)
@@ -124,16 +124,12 @@ extern "C" {
* if unaligned access is supported for integers.
*
* @since New in 1.7.
+ * @deprecated Provided for backward compatibility with the 1.14 API.
*/
-#ifndef SVN_UNALIGNED_ACCESS_IS_OK
-# if defined(_M_IX86) || defined(i386) \
- || defined(_M_X64) || defined(__x86_64) \
- || defined(__powerpc__) || defined(__ppc__)
-# define SVN_UNALIGNED_ACCESS_IS_OK 1
-# else
-# define SVN_UNALIGNED_ACCESS_IS_OK 0
-# endif
+#ifdef SVN_UNALIGNED_ACCESS_IS_OK
+# undef SVN_UNALIGNED_ACCESS_IS_OK
#endif
+#define SVN_UNALIGNED_ACCESS_IS_OK 0
]]]
