Search Unity

When is passing native collections by value not ok?

Discussion in 'Entity Component System' started by pk1234dva, Jun 3, 2022.

  1. pk1234dva

    pk1234dva

    Joined:
    May 27, 2019
    Posts:
    84
    Is it ever not safe to pass in an already allocated native collection to some function for modifications (without in/ref)? In the sense that all the modifications will be reflected in the original structure instance. Assuming I don't assign a new array to the parameter etc. of course.

    Array and list both seem fine from what I've tried, and a quick look at the source code seems to confirm this, but I'm wondering whether it's anywhere mentioned that this is guaranteed for all native collections, or whether it's at least the goal.
     
  2. Razmot

    Razmot

    Joined:
    Apr 27, 2013
    Posts:
    346
    it's always safe by design, and I can confirm it is mentioned somewhere but I don't remember where exactly.

    It's a special value type that wraps a pointer to native memory.

    You *might* even get into weird problems if you pass them by ref. I actually remember having burst complain about a in ecs dynamicbuffer once in my own code.
     
  3. Neto_Kokku

    Neto_Kokku

    Joined:
    Feb 15, 2018
    Posts:
    1,751
    It's safe, basically because the structs actually contain pointers to where their data and other attributes like list length are stored, so multiple copies of the struct all point to the same data.

    However, the Unsafe collections are not safe to pass by value. For example, the UnsafeList stores the list length in the struct itself, so if you pass it by value and add an item to the list only that copy will have the changes reflected on it (worse, if the list reallocates to increase capacity, the other copies will still point to the old memory address).
     
    Razmot likes this.
  4. Enzi

    Enzi

    Joined:
    Jan 28, 2013
    Posts:
    966
    I'd just like to clarify this because I've read it several times and it confused me when writing a NativeContainer.

    There's nothing special about a [NativeContainer] or anything special of how the job system handles NativeContainers. Every parameter is passed by value and [NativeContainer] is primarily for wrapping around safety measures.

    Wha is special is that NativeContainers are designed to have pointers inside them so when passed by value you'll get the same memory addresses.
    The best example is the NativeList that has a pointer to UnsafeList. The length of the NativeList lives in the UnsafeList so when you pass the NativeList around by value and Add an item, the length still increases.
    When the length would be in NativeList itself or NativeList wouldn't have a pointer to UnsafeList, it would stay the same when not passed by ref.

    NativeContainers can be passed by ref and should be done so when possible because there's no struct copying occuring then.

    Also, not every NativeContainer follows this pointer principle but all of them have safety measures in place. Some are expected to be passed by ref like NativeStream. It doesn't care when BeginForEachIndex hasn't been called but when it has, it explicitly checks for the ref passing. Unlike other NativeContainers, NativeStream only has the struct itself and not a pointer. This will mostly never affect anyone but when you want to start a NativeStream and then pass it to a method, you'll run into this.
     
    Razmot likes this.
  5. Guedez

    Guedez

    Joined:
    Jun 1, 2012
    Posts:
    827
    Not safe at all actually. If you pass it by value, and then Dispose one of the copies, the other will not know it was disposed and break all kinds of stuff. I don't remember which it was, but those IsCreated, IsDisposed methods get out of sync and don't work as expected.
    But if you are just going to read/write the contents and both places that uses it know for sure when it is disposed at the same time, then there should be no issues.
     
  6. lclemens

    lclemens

    Joined:
    Feb 15, 2020
    Posts:
    761
    I'm confused... Two posts say it's safe and two say that it's not safe.
     
    bb8_1 likes this.
  7. vectorized-runner

    vectorized-runner

    Joined:
    Jan 22, 2018
    Posts:
    398
    Passing NativeContainers is ok, since they store a pointer to their UnsafeContainer parts, but if you pass UnsafeContainers by value and write to it you'll break the code.

    Code (CSharp):
    1.            
    2. NativeArray<UnsafeList<int>> values;
    3.  
    4. var copy = values[0];
    5. copy[0].Add(0);
    6. // This is 1
    7. Debug.Log(copy.Length);
    8. // This is 0
    9. Debug.Log(values[0].Length);
    10.  
    11. ref var reference = UnsafeUtility.ArrayElementAsRef<UnsafeList<int>>(values.GetUnsafePtr(), 0);
    12. reference.Add(0);
    13. // This is 1
    14. Debug.Log(reference.Length);
    15. // This is also 1, you did it correctly
    16. Debug.Log(values[0].Length);
    17.  
     
    bb8_1, VietQuang, MNNoxMortem and 2 others like this.
  8. Antypodish

    Antypodish

    Joined:
    Apr 29, 2014
    Posts:
    10,778
    Depends what you do with them.
    However, you need to apply special attention to UNSAFE collections.
     
  9. pk1234dva

    pk1234dva

    Joined:
    May 27, 2019
    Posts:
    84
    Thanks for all the replies guys.

    Oh yeah, I forgot about that, and I think that's definitely confusing. There should be some easy way to tell whether a property of a native container is more at the level of the pointer, or at the level of the actual data. In c/c++ you are immediately able to tell by seeing whether "." or "->" is used, but here, you can't really tell.

    It would be nice if you could assume that methods/fields of native containers are properties of the actual data, not related to the pointer. Even changing container.Dispose() to Dispose(container) would seem a bit nicer to me. I don't even see much of a point in having the IsCreated field, at least not how it currently works - if you want some sort of tracking, make your own wrapper or something.