Search Unity

Burst - Padding in structs random only if using custom constructors

Discussion in 'Data Oriented Technology Stack' started by cjddmut, Oct 22, 2019.

  1. cjddmut

    cjddmut

    Joined:
    Nov 19, 2012
    Posts:
    120
    The snippet below reproduces the error. The short of it is if I create a struct using a custom constructor and add it to a list then the padding in that struct is garbage. This is NOT the case if I create a default struct first and set the values or just turn off Burst. This is causing some issues in our project because we assume that memory is zero (and it is without burst!).

    Code (CSharp):
    1. using Unity.Burst;
    2. using Unity.Collections;
    3. using Unity.Collections.LowLevel.Unsafe;
    4. using Unity.Jobs;
    5. using UnityEngine;
    6.  
    7. public class TestBurstStructConstructor : MonoBehaviour
    8. {
    9.     public struct TestStruct
    10.     {
    11.         public int intValue;
    12.         public byte byteValue;
    13.  
    14.         public TestStruct(int intValue, byte byteValue)
    15.         {
    16.             this.intValue = intValue;
    17.             this.byteValue = byteValue;
    18.         }
    19.     }
    20.  
    21.     // If we don't use burst then there isn't any problems
    22.     [BurstCompile(CompileSynchronously = true)]
    23.     private struct AddJob : IJob
    24.     {
    25.         public NativeList<TestStruct> list;
    26.  
    27.         public void Execute()
    28.         {
    29.             // The padding after the byte is 'random'
    30.             list.Add(new TestStruct(0, 0));
    31.            
    32.             // Below works correctly even with burst
    33.             //TestStruct ts = default;
    34.             //ts.intValue = 0;
    35.             //ts.byteValue = 0;
    36.             //list.Add(default);
    37.         }
    38.     }
    39.    
    40.     private void Update()
    41.     {
    42.         NativeList<TestStruct> list = new NativeList<TestStruct>(Allocator.TempJob);
    43.  
    44.         AddJob job = default;
    45.         job.list = list;
    46.        
    47.         job.Schedule().Complete();
    48.        
    49.         LogMemory(list);
    50.  
    51.         list.Dispose();
    52.     }
    53.  
    54.     private unsafe void LogMemory(NativeList<TestStruct> list)
    55.     {
    56.         void* ptr = list.GetUnsafeReadOnlyPtr();
    57.         int sizeInBytes = list.Length * UnsafeUtility.SizeOf<TestStruct>();
    58.        
    59.         string msg = "";
    60.  
    61.         for (int i = 0; i < sizeInBytes; i++)
    62.         {
    63.             if (i != 0 && i % 8 == 0)
    64.             {
    65.                 msg += "\n";
    66.             }
    67.  
    68.             if (i % 8 != 0 && i % 4 == 0)
    69.             {
    70.                 msg += " ";
    71.             }
    72.                
    73.             byte b = *(((byte*) ptr) + i);
    74.  
    75.             msg += b.ToString("X2") + " ";
    76.         }
    77.  
    78.         Debug.Log(msg);
    79.     }
    80. }
    Running the script above my expectation is 00 00 00 00 00 00 00 00 printed repeatedly. This is the case if I don't use burst or if I set with default. But using the custom constructor for the struct results in garbage (potentially) in the final three bytes.

    Unity: 2019.3.0b5
    Burst: preview.6 - 1.2.0

    As an aside is there a good tool to get a look at a memory location for Unity projects? Rider doesn't seem to do it and Visual Studio tells me 'unable to evaluate expression'. Seems like a tool that can is gonna become much more important for Unity development (at least if developing low level).
     
  2. xoofx

    xoofx

    Unity Technologies

    Joined:
    Nov 5, 2016
    Posts:
    246
    Padding can have garbage, there is nothing that says that it should be initialized to zero so I don't think it is a good idea to rely on that.
     
    sschoener likes this.
  3. cjddmut

    cjddmut

    Joined:
    Nov 19, 2012
    Posts:
    120
    Right now it would appear that all the other ways of constructing a struct generates zero padding.

    Code (CSharp):
    1. // Zero Padding
    2. TestStruct ts = default;
    3. ts.intValue = 0;
    4. ts.byteValue = 0;
    5. list.Add(default);
    6.  
    7. // Zero Padding
    8. list.Add(new TestStruct());
    9.  
    10. // Non-Zero Padding
    11. list.Add(new TestStruct(0, 0));
    12.  
    13. // Unless this is how I defined my struct constructor (with this())
    14. public TestStruct(int intValue, byte byteValue) : this()
    15. {
    16.     this.intValue = intValue;
    17.     this.byteValue = byteValue;
    18. }
    19.  
    20. // Now has zero padding!
    21. list.Add(new TestStruct(0, 0));
    Can these be relied on? Is it dangerous to do so in case the burst compiler changes in the future? Being able to depend on zero padding is making some tasks MUCH easier (in my cases generically checksumming a block of memory). Also, as mentioned, if I don't create a struct in burst then it has zero padding in all cases.
     
  4. eizenhorn

    eizenhorn

    Joined:
    Oct 17, 2016
    Posts:
    1,692
    On all platforms or only on your dev machine? ;)
     
  5. cjddmut

    cjddmut

    Joined:
    Nov 19, 2012
    Posts:
    120
    In at least PC Standalone on multiple computers. Really seems more of a compiler question (or .net question in the case of non-burst) then machine dependent.
     
  6. eizenhorn

    eizenhorn

    Joined:
    Oct 17, 2016
    Posts:
    1,692
    It's what I meant, agree wasn't clear. My point was - compiler depends on choosed platform and padding can be different :)
     
  7. cjddmut

    cjddmut

    Joined:
    Nov 19, 2012
    Posts:
    120
    After investigating further I can come up with structs that end up with garbage in all burst scenarios. The original one worked cause the compiler was setting an immediate qword which happened to zero out the garbage. However the following struct can have garbage in any case.

    Code (CSharp):
    1. public struct TestStruct
    2. {
    3.     public int intValue;
    4.     public byte byteValue2;
    5.     public int intValue2;
    6.     public byte byteValue;
    7.  
    8.     public TestStruct(int intValue, byte byteValue)
    9.     {
    10.         this.intValue = intValue;
    11.         this.byteValue = byteValue;
    12.         this.intValue2 = 0;
    13.         this.byteValue2 = 0;
    14.     }
    15. }
    Which will lead me to a follow up question. I wanted consistent padding cause I found it awfully convenient to be able to checksum an entire block of memory (our game requires determinism so we checksum state to ensure we haven't diverged). So long as a struct was simple (blittable with no pointers) then I had hoped to not need a custom checksum built for it and could just checksum the array of structs quickly.

    What would the suggested path forward be? I could consider not using burst assuming mono (and IL2CPP for consoles) does zero out the padding but for the most part burst has been pretty amazing. I could bite the bullet and require our devs to write checksums for all structs even though 95% of them are simple. Is there a good route I'm not seeing here?
     
    Last edited: Oct 23, 2019
  8. tertle

    tertle

    Joined:
    Jan 25, 2011
    Posts:
    1,956
    It sounds like this is just for validation. Does it need to be super performant and execute at runtime?

    If not I guess 1 solution could be to just serialise the struct and checksum that output.
     
    cjddmut likes this.
  9. cjddmut

    cjddmut

    Joined:
    Nov 19, 2012
    Posts:
    120
    It's true that we could turn it off for release and not care as much about performance. The idea that it was always on was a little enticing cause I'm sure some determinism issues might sneak through and detecting them to disconnect the player and have them rejoin (or resync) sounded better than just letting the simulation diverge.

    Serializing it first is an idea I'll have to explore.
     
unityunity