Better to avoid vdpps due to high latency

[asenkew]

Hello Jorrit,
here is attached the patch giving performance gain for x86.
patch_jolt_avoid_vdpps.txt
Measured on AMD and Intel HW, both have uplifts.
Part of results on Ryzen 9 9950X (got with -s=Pyramid -q=Discrete -repeat=10),
columns are Threads number and Speedup:
1 8.80%
2 6.66%
3 6.48%
4 5.92%
5 5.55%
6 5.01%
7 4.63%
8 3.91%
9 1.75%
10 -0.40%
11 -0.85%
12 -0.77%
13 -0.67%
14 -0.82%
15 -0.51%
16 -0.47%

Full measurements are attached in to keep this thread cleaner.
avoid_vdpps_measurements.txt

Let me know please what you think and how to proceed to have the fix merged.

[jrouwe]

I've done some quick testing and it indeed seems to be a win. One thing to be aware of is that if you change the code, you usually also change the result of the sim. E.g. from compiling with MSVC 2026 I got with the old code:

Discrete, 1, 93.330482, 0x8279f8daecc52bb5

with the new code:

Discrete, 1, 96.171926, 0xf9d87c53ef19cd61

so you see that the hash changed, which can make quite a bit of difference in performance (e.g. because a stack falls over spreading bodies out instead of staying stacked).

You can specify the CROSS_PLATFORM_DETERMINISTIC cmake option to make the sim the same:

Discrete, 1, 89.880822, 0xafd93b295e75e3f6

vs new:

Discrete, 1, 93.612736, 0xafd93b295e75e3f6

which is slower in both cases, but now we're at least comparing the same sim (difference is +4% for the new code @ 1 thread).

I need to do some more testing, but if you could create a PR for this change then at least you'll get credit when I merge it.

[jrouwe]

When compiling with MSVC / clang, fp-contract is off when cross platform determinism is required, on otherwise. Gcc has it off always:

JoltPhysics/Build/CMakeLists.txt

Lines 205 to 210 in 47be86b

	if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "MSVC")
	if (CROSS_PLATFORM_DETERMINISTIC)
	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /fp:precise")
	else()
	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} /fp:fast") # Clang doesn't use fast math because it cannot be turned off inside a single compilation unit
	endif()

JoltPhysics/Build/CMakeLists.txt

Lines 246 to 264 in 47be86b

	if ("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
	# Also disable -Wstringop-overflow or it will generate false positives that can't be disabled from code when link-time optimizations are enabled
	# Also disable -Wno-psabi to avoid messages of the form note: parameter passing for argument of type '...' changed in GCC 7.1
	# Also turn off automatic fused multiply add contractions, there doesn't seem to be a way to do this selectively through the macro JPH_PRECISE_MATH_OFF
	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-stringop-overflow -Wno-psabi -ffp-contract=off")
	else()
	# Do not use -ffast-math since it cannot be turned off in a single compilation unit under clang, see Core.h
	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ffp-model=precise")
	# On clang 14 and later we can turn off float contraction through a pragma, older versions and deterministic versions need it off always, see Core.h
	# clang on LoongArch does not support such pragma, also turn off contraction for it.
	if (CMAKE_CXX_COMPILER_VERSION LESS 14 OR CROSS_PLATFORM_DETERMINISTIC OR "${CMAKE_SYSTEM_PROCESSOR}" MATCHES "loongarch")
	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ffp-contract=off")
	# Clang 20 and later complain with: overriding '-ffp-model=precise' option with '-ffp-contract=off' [-Woverriding-option], but this is exactly what we want.
	if (CMAKE_CXX_COMPILER_VERSION GREATER 19)
	set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-overriding-option")
	endif()
	endif()

I'm not sure how much difference there is between 'fast' and 'on', but we could potentially use 'fast' when cross platform determinism is off.

[asenkew]

Hello Jorrit, with 'fast' compilers do more aggressive FMAs emission, the option has mostly positive impact on all 10 test (5 in 2 modes), except Ragdoll Discrete having degradation.
It is accuracy impact of 'fast' because FMA computes a * b + c with a single rounding instead of two and this is actually more accurate, so whose things under if (inLambda != 0.0f) are executed more times (confirmed with added counters there).
Adding tolerance in 3 important places below helps to fix degradation:

diff --git a/Build/CMakeLists.txt b/Build/CMakeLists.txt
index db1069d7..c76865eb 100644
--- a/Build/CMakeLists.txt
+++ b/Build/CMakeLists.txt
@@ -264,6 +264,11 @@ else()
 			if (CMAKE_CXX_COMPILER_VERSION GREATER 19)
 				set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-overriding-option")
 			endif()
+		else()
+            set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -ffp-contract=fast")
+            if (CMAKE_CXX_COMPILER_VERSION GREATER 19)
+                set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -Wno-overriding-option")
+			endif()
 		endif()
 
 		# Cross compiler flags
diff --git a/Jolt/Physics/Constraints/ConstraintPart/AngleConstraintPart.h b/Jolt/Physics/Constraints/ConstraintPart/AngleConstraintPart.h
index f7493102..325dc5f8 100644
--- a/Jolt/Physics/Constraints/ConstraintPart/AngleConstraintPart.h
+++ b/Jolt/Physics/Constraints/ConstraintPart/AngleConstraintPart.h
@@ -39,7 +39,11 @@ class AngleConstraintPart
 	JPH_INLINE bool				ApplyVelocityStep(Body &ioBody1, Body &ioBody2, float inLambda) const
 	{
 		// Apply impulse if delta is not zero
+#ifdef JPH_CROSS_PLATFORM_DETERMINISTIC
 		if (inLambda != 0.0f)
+#else
+		if (fabs(inLambda) > FLT_EPSILON)
+#endif
 		{
 			// Calculate velocity change due to constraint
 			//
diff --git a/Jolt/Physics/Constraints/ConstraintPart/AxisConstraintPart.h b/Jolt/Physics/Constraints/ConstraintPart/AxisConstraintPart.h
index 66373fb4..f139c4bb 100644
--- a/Jolt/Physics/Constraints/ConstraintPart/AxisConstraintPart.h
+++ b/Jolt/Physics/Constraints/ConstraintPart/AxisConstraintPart.h
@@ -44,7 +44,11 @@ class AxisConstraintPart
 	JPH_INLINE bool				ApplyVelocityStep(Body &ioBody1, Body &ioBody2, Vec3Arg inWorldSpaceAxis, float inLambda) const
 	{
 		// Apply impulse if delta is not zero
+#ifdef JPH_CROSS_PLATFORM_DETERMINISTIC
 		if (inLambda != 0.0f)
+#else
+		if (fabs(inLambda) > FLT_EPSILON)
+#endif
 		{
 			// Calculate velocity change due to constraint
 			//
diff --git a/Jolt/Physics/Constraints/ConstraintPart/ContactConstraintPart.h b/Jolt/Physics/Constraints/ConstraintPart/ContactConstraintPart.h
index f48bde28..5d6d696f 100644
--- a/Jolt/Physics/Constraints/ConstraintPart/ContactConstraintPart.h
+++ b/Jolt/Physics/Constraints/ConstraintPart/ContactConstraintPart.h
@@ -96,7 +96,11 @@ private:
 	/// See AxisConstraintPart::ApplyVelocityStep
 	JPH_INLINE bool				ApplyVelocityStep(Vec3 &ioLinearVelocity1, Vec3 &ioAngularVelocity1, Vec3 &ioLinearVelocity2, Vec3 &ioAngularVelocity2, float inInvMass1, float inInvMass2, Vec3Arg inWorldSpaceAxis, float inLambda) const
 	{
+#ifdef JPH_CROSS_PLATFORM_DETERMINISTIC
 		if (inLambda != 0.0f)
+#else
+		if (fabs(inLambda) > FLT_EPSILON)
+#endif
 		{
 			if constexpr (Type1 == EMotionType::Dynamic)
 			{

What do you think about this patch?

1 thread x86 improvements on AMD 9950X:
Ragdoll 9%, 5%
Pyramid 3%, 2%
RagdollSinglePile 5%, 3%
ConvexVsMesh 3%, 2%
CharacterVirtual 1.7%, 2.5%
and better with more threads.
I can add more measurements if such fix is feasible.

[jrouwe]

I've tried you patch with clang 20.1.8 but this makes several unit tests fail:

Single precision x86 64-bit with instructions: SSE2 SSE4.1 SSE4.2 AVX AVX2 F16C LZCNT TZCNT FMADD (16-bit ObjectLayer) (ObjectStream)
[doctest] doctest version is "2.4.12"
[doctest] run with "--help" for options
===============================================================================
/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec3Tests.cpp:392:
TEST SUITE: Vec3Tests
TEST CASE:  TestVec3CompressUnitVector

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec3Tests.cpp:395: ERROR: CHECK( Vec3::sDecompressUnitVector(Vec3::sAxisX().CompressUnitVector()) == Vec3::sAxisX() ) is NOT correct!
  values: CHECK( 1, -1.97542e-08, -1.97542e-08 == 1, 0, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec3Tests.cpp:396: ERROR: CHECK( Vec3::sDecompressUnitVector(Vec3::sAxisY().CompressUnitVector()) == Vec3::sAxisY() ) is NOT correct!
  values: CHECK( -1.97542e-08, 1, -1.97542e-08 == 0, 1, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec3Tests.cpp:397: ERROR: CHECK( Vec3::sDecompressUnitVector(Vec3::sAxisZ().CompressUnitVector()) == Vec3::sAxisZ() ) is NOT correct!
  values: CHECK( -1.97542e-08, -1.97542e-08, 1 == 0, 0, 1 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec3Tests.cpp:398: ERROR: CHECK( Vec3::sDecompressUnitVector((-Vec3::sAxisX()).CompressUnitVector()) == -Vec3::sAxisX() ) is NOT correct!
  values: CHECK( -1, 1.97542e-08, 1.97542e-08 == -1, 0, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec3Tests.cpp:399: ERROR: CHECK( Vec3::sDecompressUnitVector((-Vec3::sAxisY()).CompressUnitVector()) == -Vec3::sAxisY() ) is NOT correct!
  values: CHECK( 1.97542e-08, -1, 1.97542e-08 == 0, -1, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec3Tests.cpp:400: ERROR: CHECK( Vec3::sDecompressUnitVector((-Vec3::sAxisZ()).CompressUnitVector()) == -Vec3::sAxisZ() ) is NOT correct!
  values: CHECK( 1.97542e-08, 1.97542e-08, -1 == 0, 0, -1 )

===============================================================================
/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec4Tests.cpp:785:
TEST SUITE: Vec4Tests
TEST CASE:  TestVec4CompressUnitVector

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec4Tests.cpp:788: ERROR: CHECK( Vec4::sDecompressUnitVector(Vec4(1, 0, 0, 0).CompressUnitVector()) == Vec4(1, 0, 0, 0) ) is NOT correct!
  values: CHECK( 1, 1.90921e-08, 1.90921e-08, 1.90921e-08 == 1, 0, 0, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec4Tests.cpp:789: ERROR: CHECK( Vec4::sDecompressUnitVector(Vec4(0, 1, 0, 0).CompressUnitVector()) == Vec4(0, 1, 0, 0) ) is NOT correct!
  values: CHECK( 1.90921e-08, 1, 1.90921e-08, 1.90921e-08 == 0, 1, 0, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec4Tests.cpp:790: ERROR: CHECK( Vec4::sDecompressUnitVector(Vec4(0, 0, 1, 0).CompressUnitVector()) == Vec4(0, 0, 1, 0) ) is NOT correct!
  values: CHECK( 1.90921e-08, 1.90921e-08, 1, 1.90921e-08 == 0, 0, 1, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec4Tests.cpp:791: ERROR: CHECK( Vec4::sDecompressUnitVector(Vec4(0, 0, 0, 1).CompressUnitVector()) == Vec4(0, 0, 0, 1) ) is NOT correct!
  values: CHECK( 1.90921e-08, 1.90921e-08, 1.90921e-08, 1 == 0, 0, 0, 1 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec4Tests.cpp:792: ERROR: CHECK( Vec4::sDecompressUnitVector(Vec4(-1, 0, 0, 0).CompressUnitVector()) == Vec4(-1, 0, 0, 0) ) is NOT correct!
  values: CHECK( -1, -1.90921e-08, -1.90921e-08, -1.90921e-08 == -1, 0, 0, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec4Tests.cpp:793: ERROR: CHECK( Vec4::sDecompressUnitVector(Vec4(0, -1, 0, 0).CompressUnitVector()) == Vec4(0, -1, 0, 0) ) is NOT correct!
  values: CHECK( -1.90921e-08, -1, -1.90921e-08, -1.90921e-08 == 0, -1, 0, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec4Tests.cpp:794: ERROR: CHECK( Vec4::sDecompressUnitVector(Vec4(0, 0, -1, 0).CompressUnitVector()) == Vec4(0, 0, -1, 0) ) is NOT correct!
  values: CHECK( -1.90921e-08, -1.90921e-08, -1, -1.90921e-08 == 0, 0, -1, 0 )

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Math/Vec4Tests.cpp:795: ERROR: CHECK( Vec4::sDecompressUnitVector(Vec4(0, 0, 0, -1).CompressUnitVector()) == Vec4(0, 0, 0, -1) ) is NOT correct!
  values: CHECK( -1.90921e-08, -1.90921e-08, -1.90921e-08, -1 == 0, 0, 0, -1 )

===============================================================================
/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Physics/CharacterVirtualTests.cpp:233:
TEST SUITE: CharacterVirtualTests
TEST CASE:  TestMovingOnSlope

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/Build/../UnitTests/UnitTestFramework.h:37: ERROR: CHECK( inLHS.IsClose(inRHS, inTolerance * inTolerance) ) is NOT correct!
  values: CHECK( false )

===============================================================================
/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Physics/ShapeTests.cpp:840:
TEST SUITE: ShapeTests
TEST CASE:  TestMeshShapePerTriangleUserData

/mnt/c/Users/jrouw/Documents/Code/JoltPhysics/UnitTests/Physics/ShapeTests.cpp:885: ERROR: CHECK( collector.mHits.size() == triangles[0].size() + triangles[1].size() ) is NOT correct!
  values: CHECK( 799 == 800 )

===============================================================================
[doctest] test cases:    573 |    569 passed |  4 failed | 0 skipped
[doctest] assertions: 511049 | 511033 passed | 16 failed |
[doctest] Status: FAILURE!

The problem appears to be that this code:

JoltPhysics/Jolt/Core/Core.h

Lines 615 to 619 in 05fcfbf

	#define JPH_PRECISE_MATH_ON \
	_Pragma("float_control(precise, on, push)") \
	_Pragma("clang fp contract(off)")
	#define JPH_PRECISE_MATH_OFF \
	_Pragma("float_control(pop)")

Is unable to turn off floating point contractions when using -ffp-contract=fast. You can verify this by doing:

gdb --args ./PerformanceTest -q=LinearCast -s=Pyramid -t=1
b JPH::ClosestPoint::GetBaryCentricCoordinates
r
disassemble

and observing that the function is full of FMA instructions even though it is surrounded by JPH_PRECISE_MATH_ON / JPH_PRECISE_MATH_OFF. This triggers precision problems in the many a * b - c * d constructs in that function because the precision of a * b and c * d is different with FMA.

W.r.t. using if (fabs(inLambda) > FLT_EPSILON): This slows down the simulation with approx 2% when using MSVC. It is something that I tried before but abandoned because of the loss. I'm not too worried about 1 test having slightly degraded performance when using 'fast' contraction mode though if all other tests are faster.

[jrouwe]

To add to this: A simple repro in godbolt shows that clang indeed ignores #pragma clang fp contract(off) when -ffp-contract=fast is used (it works when -ffp-contract=on is used).

Note that there is Kahan's algorithm that could be used to improve the accuracy of the difference of products, which would allow us to not turn FMA off in the scope of these functions. So far, I've shied away from using it because it would mean that hardware without FMA would have less accurate collision detection. Since I always have FMA on, I might miss problems that way.

[jrouwe]

I managed to enable -ffp-contract=fast on x86_64 in #2021 (which builds upon #1998, #1999, #2002 and #2020). I didn't manage to enable it on ARM because when you compile without optimizations, it actively removes FMA instructions which causes precision issues. Add -O3 or use -ffp-contract=on and it will work. On macOS it also doesn't work with -O3 for reasons unknown to me. I suspect it's related to link time optimization because the code it generates with -flto=thin is different between 'on' and 'fast' mode.

[jrouwe]

To answer your other question:

How performance should be measured? I mean is -repeat=10 -q=Discrete enough?

This is in general what I do too (make sure you use a Distribution build), but keep in mind what I said above about the simulation changing when you change code.