Search code examples
c#unsafe

Is it within best practices to access fields by their memory address?

I recently bought the book, Foundations of Game Engine Development, Volume 1: Mathematics, and all of the code examples in it are C++.

In the C++ implementation of Vector3D, the author created an indexer like this:

struct Vector3D
{
    float x, y, z;

    float& operator [](int i)
    {
        return ((&x)[i]);
    }
}

So,the C# equivalent be:

struct Vector3D
{
    float x, y, z;

    public float this[int index]
    {
        get
        {
            fixed (float* x = &X)
            {
                return x[index];
            }
        }
    }
}

Is this allowed? In C++ this is undefined behavior, so I was wondering if this would be ill-advised to do in C#. This is definitely unsafe, but, if the index is 0, 1 or 2, will the behavior be consistent?

Here is the full source code.

Solution

  • Is it within best practices to access fields by their memory address?

    In short: no. No, it is not a best-practice to do this. There are no performance benefits and you lose compiler-enforced type-safety. Doing this will likely harm performance because the compiler cannot make as many assumptions to optimize the compiled program. Never try to outsmart the compiler.

    In the C++ implementation of Vector3D, the author created an indexer like this:

    The C++ code is doing two different things btw:

    1. It's returning a float&.

      • Note that a float& is not a pointer or a "raw" memory address (internally it may be represented by a pointer, but references in C++ can be transformed into some pretty funky native instruction logic when you have a good compiler).
      • In C# you can return a float& (actually ref float in C#) to an array element or a field.

    2. It's assuming the struct's fields will be laid-out sequentially with the same padding as array elements.

      • This is contrary to the C++ language specification:
        • The C++ specification only guarantees that elements in an array are contiguous (and sizeof() includes the necessary padding). This guarantee is not extended to struct/class fields, even when they are of uniform type.
      • In C#, the compiler will actively prevent you writing code that makes this assumption.
      • There is no real practical C# equivalent of that C++'s code (&x)[i] expression - because it's just wrong.
      • It's wrong because because the struct Vector3D lacks #pragma directives to control the struct-packing.

    So this C++:

    class Foo
{
    int x[3];
}

    Is not guaranteed to have the same exact in-memory representation as this:

    class Bar
{
    int x0;
    int x1;
    int x2;
}

    This is explained in this QA here: Layout in memory of a struct. struct of arrays and array of structs in C/C++

    (While it's likely on a typical x86 computer that they'll "just work" - it's also just as possible that the indexer is reading and writing to parts of memory that don't correspond to the array elements - but because C++ doesn't have automatic runtime bounds-checking when using raw pointers you won't know your code is corrupting your process' memory by overwriting memory incorrectly until it's too late (if you're lucky, the OS or runtime memory allocator might detect something is wrong and terminate). These are the same kinds of assumptions that horrible C++ programmers used in the 32-bit world, such as assuming that sizeof(int*) == sizeof(int) - and we all had fun with that when we moved to x64 in the mid-2000s)

    Now, for what it's worth, C# does allow its users a limited amount of type-punning, even without the unsafe modifier, by use of [FieldOffset] attributes (this is how you can define a union in C# for compatibility with native APIs: by using overlapping offsets) - but this comes at a performance cost (somewhat counter-intuitively: smaller and efficiently packed structures are slower to process because of native word alignment issues).

    In an unsafe context, I believe the C# equivalent to the C++ would be this (I probably have the indexer wrong, it's been literally 5-6 years since I last had to use unsafe C# code):

    struct Vector3D
{
    [FieldOffset( sizeof(float) * 0 )]
    private float x;

    [FieldOffset( sizeof(float) * 1 )]
    private float y;

    [FieldOffset( sizeof(float) * 2 )]
    private float z;

    public unsafe float* this[Int32 i]
    {
        get
        {
            float* x0 = &this.x;
            return &x0[i];
        }
    }
}

    And this is wrong so so many levels:

    1. Interpreting &this.x as a pointer to an array of fields-by-index will not be faster than simply using a switch - which will also be safer.
    2. There's no bounds-checking.
    3. Using FieldOffset and struct-packing actually makes programs slower because values will not be aligned on a CPU word-boundary.

    If you want a fast and safe float 3-vector in C#, do this:

    struct Vector3
{
    public float x;
    public float y;
    public float z;

    public ref float this[Int32 i]
    {
        get
        {
            switch( i )
            {
            case 0: return ref this.x;
            case 1: return ref this.y;
            case 2: return ref this.z;
            default: throw new ArgumentOutOfRangeException( nameof(i) );
            }
        }
    }
}

    You can do the same thing in C++ too and will still very likely get better performance than the original Vector3D class because the compiler can optimize direct named-field accesses better compared to manipulating raw memory via a field-offset pointer.

    Update

    In response to a question in a comment reply by the OP:

    Would it make sense to use a fixed buffer as a backing field for X, Y and Z? That would eliminate the need for a switch statement. Also, on the topic of mutable structs, with the new-ish feature of readonly structs, is there really any time when a struct should not be readonly?

    There is no reason to use a buffer (fixed or otherwise) for this use-case. Because this is a Vector3 there will only ever be 3 elements, so it should only use fields (if you use a heap-allocated array then you lose Locality of Reference benefits.

    In summary: there are no advantages to the original approach and plenty of disadvantages (such as the lack of bounds-checking, which is very, very important for memory-safety). Note that an integer switch is compiled to a very fast native jump-table directly in machine-code, making it effectively a zero-cost language feature.