Search Unity

Deallocating memory.

Discussion in 'Entity Component System' started by pk1234dva, Dec 1, 2021.

  1. pk1234dva

    pk1234dva

    Joined:
    May 27, 2019
    Posts:
    84
    I've got a relatively lengthy method that schedules lots of jobs, each depending on the previous job. If all goes according to plan, I should be easily able to simply Dispose all the necessary objects. However, if any exception were to happen at any place, the situation becomes complicated.

    Is there any good advice on how to handle such situations?

    Here's an example of a recent problem I've considered: suppose job B is dependent on job A, both using some NativeArray. If something goes wrong during scheduling of B and an exception is thrown, I now have no way of checking whether A is completed. So I have no way of checking whether I can dispose of the array. What do I do when something like this happens?

    One possible solution I've considered, is that I could have a single function that attempts to dispose of all the objects, in the case an exception was thrown and something failed. That itself seems problematic though - what if the array wasn't allocated properly? What if the array already was disposed of? There doesn't seem to be a good way of checking besides using a try/catch and catch the double disposal attempt.
     
  2. DreamingImLatios

    DreamingImLatios

    Joined:
    Jun 3, 2017
    Posts:
    4,255
    Don't you still have the JobHandle for A? Can't you just complete that?
     
  3. Guedez

    Guedez

    Joined:
    Jun 1, 2012
    Posts:
    827
    If you don't have job handles, you can use this:
    Code (CSharp):
    1.  
    2.     public class DisposableCollection<T> where T : struct, IDisposable {
    3.         public T ToDispose;
    4.         public bool Disposed;
    5.         public DisposableCollection(T ToDispose) {
    6.             this.ToDispose = ToDispose;
    7.             Disposed = false;
    8.         }
    9.         public T GetDisposable() {
    10.             return ToDispose;
    11.         }
    12.     }
    13.     public static class DisposableCollectionExtensions {
    14.         public static void TryDispose<T>(this DisposableCollection<T> AThis) where T : struct, IDisposable {
    15.             if (AThis != null) {
    16.                 if (!AThis.Disposed) {
    17.                     AThis.Disposed = true;
    18.                     AThis.ToDispose.Dispose();
    19.                 }
    20.             }
    21.         }
    22.         public static bool Valid<T>(this DisposableCollection<T> AThis) where T : struct, IDisposable {
    23.             if (AThis != null) {
    24.                 if (AThis.Disposed) {
    25.                     return false;
    26.                 }
    27.                 return true;
    28.             }
    29.             return false;
    30.         }
    31.     }
    Dispose and Valid are made through an extension method so you can just use it instead of needing to nullcheck, I should've made
    ToDispose
    access through an extension method too now that I think of it.
    Also note that this will not work if your collections are disposed elsewhere; All that this does is to enable you to "TryDispose" the same collection multiple times. The ideal solution is to not use this at all and rethink what you are doing so you don't need to "TryDispose" at all, but I am short for time so I didn't.
    This is completely not burst compatible, and is a gigantic clutch rather than a solution. The correct way is to ensure that your code have no bugs that can cause exceptions and thus ensure you will always dispose of all disposables once and only once. Which is a ludicrous idea as the whole point of bugs is being there when you least expect them.
     
  4. pk1234dva

    pk1234dva

    Joined:
    May 27, 2019
    Posts:
    84
    That's a possibility, but if I've got jobs a, b, c, d, e etc. that are all dependent on the previous one, I'd rather not have to store the job handle for each one as I schedule them, and then check all the scheduled ones have completed, before disposing of all that's necessary. I'd think there's a better way.
     
  5. pk1234dva

    pk1234dva

    Joined:
    May 27, 2019
    Posts:
    84
    Thanks. Yes, I've considered something like this - simply keeping track of what has been allocated/deallocated manually, though it seems a bit weird. I guess alternatively, I could just make sure to work with a the NativeArrays etc. as fields of a class, only passing them by reference, etc - then I could rely on IsCreated.

    I guess there's not much that can be done about this in general. Maybe checking if there's a valid header or so in native memory above the memory address of the pointer could be possible, but there would first have to be a check whether the address is accessible etc, and I'm not sure whether that's something expensive to check or something like that. Doesn't sound like a bad idea to me, but I'm no low level programmer.

    Keeping track of allocation still doesn't really deal with the issue of knowing whether I can even attempt a dispose - I need to know whether the jobs that are using it are running.
     
  6. DreamingImLatios

    DreamingImLatios

    Joined:
    Jun 3, 2017
    Posts:
    4,255
    You only need to store the last JobHandle in each chain before you enter the block of code that can potentially throw an exception. As a general rule you should quarantine the block of code that can throw exceptions and keep job scheduling outside of it. Use bools and if statements to control flow jobs that shouldn't be scheduled.
     
  7. pk1234dva

    pk1234dva

    Joined:
    May 27, 2019
    Posts:
    84
    Thanks. Yes, you are correct, just the last job handle is enough.

    I'm not actually throwing any of my own exceptions, and I'm not really aware of any place where an exception could be thrown. Just trying to be careful and make sure there's no terrible consequences if something bad happens - memory leaks, hard crashes, etc.
     
  8. DreamingImLatios

    DreamingImLatios

    Joined:
    Jun 3, 2017
    Posts:
    4,255
    Unless you are working with a third-party library that was designed to throw exceptions, you should never need to check for them, and checking for them has a performance impact. You are better off spending your time testing your code than designing for exceptions which may or may not exist and may or may not be recoverable.
     
  9. pk1234dva

    pk1234dva

    Joined:
    May 27, 2019
    Posts:
    84
    Could you provide a source or reasoning behind some of this? I'm sure exceptions have some extra cost, but I'm under the impression this is something that has been largely exaggerated.

    Clearly catching exceptions frequently / using them intentionally for control flow is a bad idea, but for some actions that are done infrequently, I'm not sure it's worth caring about. I'm also not sure there's even much of a cost in the case that no exception is ever actually thrown, and that there's much overhead to having a simple try + finally block, for example, if no exception is actually thrown inside the try block.

    EDIT:
    https://www.jacksondunstan.com/articles/3368
    Tests in this article seem to suggest that the overhead of checking for exceptions is very low, on scale with no-ops.
     
    Last edited: Dec 2, 2021
  10. DreamingImLatios

    DreamingImLatios

    Joined:
    Jun 3, 2017
    Posts:
    4,255
    Yeah. It probably doesn't mean much performance-wise in Unity's slow Mono runtime. If this is a MonoBehaviour-based project and you don't plan on Bursting your job scheduling code in the future, then really the downside focuses more on development effort. Writing code to work around a non-existent exception is a waste of development effort which makes your code more complicated and harder to maintain. And writing code around an exception that exists but you don't understand just means you are hiding bugs. It is an anti-pattern. How do you know that it is safe to continue running the application after encountering some unexpected exception? And if it isn't, my guess is you might try to save some state and quit the app, in which case a memory leak isn't that big of a deal since the process will stop soon and the OS will clean it up.
     
  11. pk1234dva

    pk1234dva

    Joined:
    May 27, 2019
    Posts:
    84
    I'm definitey not planning on handling the actual exception - if it happens, either I'll rethrow it, or I might not even have a catch block in the first place. My main goal is to make sure whatever memory is allocated is freed up, so just try and finally would be used.

    The question of what to do if some unknown exception shows up, seems like a different, independent topic. I should mention that I'm working on a plugin that's not necessarily something only I will use, so I'd think that in some sort of release version, it makes sense that you don't terminate the app on such an occasion, especially if you have reason to believe that you won't cause some type of corruption. Also seems to be the default behaviour, unless you're using IL2CPP with "Fast but no exceptions" or so. So it seems to me like it's reasonable to let the user of the plugin determine what unhandled exceptions do in general, and all I do is give some attempt at cleaning up what I can.

    You might be right though, because I know I have an ocd issue and I'm quite likely wasting time trying to prepare for weird scenarios that will likely never happen, and if they do, there might be bigger issues than a memory leak.

    Thanks a ton for all the comments by the way, gives me a lot to think about.