Unity-Technologies / Unity-Technologies/Unity.Mathematics

Considerable performance fixes

Open
#66 5 comments 4 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
C#
Stars
1.4k
Forks
159
PR merge metrics
No merged PRs in 30d

Description

Hi there!
I noticed two problems that reduce performance and can easily be fixed 😄

1. Not using throw helpers

The jit will never inline a method if it throws an exception, even if you add the AgressiveInlining attribute.

Example: int2.gen.cs line:775

The common fix for this is very simple, just use a "throw helper", like this:

[MethodImpl(MethodImplOptions.AggressiveInlining)]
internal static int select_shuffle_component(int2 a, int2 b, ShuffleComponent component)
{
    switch(component)
    {
        case ShuffleComponent.LeftX:
            return a.x;
        case ShuffleComponent.LeftY:
            return a.y;
        case ShuffleComponent.RightX:
            return b.x;
        case ShuffleComponent.RightY:
            return b.y;
        default:
            ThrowInvalidArgument();
    }
}

static void ThrowInvalidArgument()
{
    throw new System.ArgumentException("Invalid shuffle component: " + component);
}

2. Using the "fixed" statement instead of a reinterpret cast

The code used for indexers can be improved a lot!
Example: int2.gen.cs line:616

The fixed statement is horrible because just like throwing exceptions, it prevents a ton of optimizations (it is implemented that way on all runtimes I tested!).

Take a look at the asm it generates:

Code:
example code

(z = test1[6] is there on purpose, not a typo)

Assembly:
asm old

This should instead be done using a reinterpret cast.

Keep in mind that even though the syntax of Unsafe can sometimes (fortunately not here) become really unwieldy, it pretty much always compiles down to very simple instructions. (In fact methods like Unsafe.As<TFrom, TTo>(ref source) are literally a nop because they only trick the C# compiler into accepting the type change).
It's really just the C# name for reinterpret_cast.

By simply casting ref this to a little helper struct all overhead is gone.

Btw Unsafe is defined in System.Runtime.CompilerServices.Unsafe. The As() method I'm using here has an extremely simple definition (in this case it does literally nothing as you can see). All other functions in there are just as trivial, so they should be really easy for Unity to add 👍

Here is what I did instead:


struct int2
{
	public int x;
	public int y;

	public unsafe int this[int index]
	{
		get
		{
			return Unsafe.As<int2, ArrayUnion>(ref this).ints[index];
		}
		set
		{
			Unsafe.As<int2, ArrayUnion>(ref this).ints[index] = value;
		}
	}
}

[StructLayout(LayoutKind.Explicit)]
internal unsafe struct ArrayUnion
{
	// The fixed array sizes are assuming a float4(x,y,z,w) as maximum
	// but their size doesn't even actually matter because we're only using
	// it to reinterpret the bits anyway!
	[FieldOffset(0)]
	public fixed int ints[4];
	[FieldOffset(0)]
	public fixed uint uints[4];
	[FieldOffset(0)]
	public fixed float floats[4];
	[FieldOffset(0)]
	public fixed short shorts[2 * 4]; // 2 shorts per field, 4 times
	[FieldOffset(0)]
	public fixed bool bools[4 * 4]; // 4 bools per field, 4 times
	// ...
}

And with the exact same C# code, this time the disassembly looks like this:

asm improved

This time the jit was able to absolutely decimate the code! Nice! Let me know what you think 😄

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with the referenced src/Unity.Mathematics/int2.gen.cs locations around lines 616 and 775, then identify how broadly the two proposed patterns apply across the generated vector types. Compare the resulting generated assembly with the examples in the issue and confirm that the affected indexers and exception paths retain their intended behavior.

Written by the indexing model from the issue text.

Assessment

Tech stack
csharp
Domain
performance
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
30/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.