Search Unity

  1. Unity 2019.2 is now released.
    Dismiss Notice

Physics.Contacts GC activity

Discussion in 'Physics' started by fallFlat, Nov 23, 2015.

  1. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    Here's how it's presented as a simple check-box that'll be on by default. The reason this was reluctantly added as an option rather than as a hidden behaviour is because there are some devs who store away the Collision/Collision2D types so recycling them would cause issues for them but they are not the majority.

    2D/3D Physics Settings

    Picture or it didn't happen I guess: https://gyazo.com/0f6d884b296976529ed3cb24f34552e6
     
    Last edited: Oct 22, 2018
    Seb-1814, spajus and JustAnotherDude like this.
  2. fallFlat

    fallFlat

    Joined:
    Mar 26, 2014
    Posts:
    24
    So this would simply pool the structures behind the scenes when enabled. That's elegant - one can get the benefits without a need to rewrite any code. Looking forward to it and thanks!
     
  3. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    It actually uses (recycles) a single instance of Collision/Collision2D and just populates it for each callback, not keep a strong reference to all Collision/Collision2D sent out because those would grow without limit. This method does mean there are no bad performance/memory implications either which is the best part.
     
  4. fallFlat

    fallFlat

    Joined:
    Mar 26, 2014
    Posts:
    24
    What about Collision.contacts? It's a non fixed length array. What happens accessing Collision.contacts? Will it trigger allocation each time accessed, once per frame, or will it be reused somehow between frames? Maybe some other, non-allocating, API will be made?
     
  5. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    We don't have that issue for 2D but for 3D yes it's still a problem that needs to be addressed by the 3D team as we cannot simply cache it here because as you say, it's not a fixed length. Will update you when I know.

    We could certainly cache a List<> which is one option in my mind.
     
    hippocoder likes this.
  6. fallFlat

    fallFlat

    Joined:
    Mar 26, 2014
    Posts:
    24
    Without changing any of the API it's hard - it requires multiple cached arrays of different sizes - but it should not get too big: you rarely will get 10s of contact points - and games with such geometry would unlikely rely on specific contact information anyway (or at least accept the involved implications if they do).
    Without sticking to current API, caching a List<>, exposing GetContacts(List<Contact>) or even making GetContactsNonAlloc(Contact[]) are all viable solutions.
     
  7. hippocoder

    hippocoder

    Digital Ape Moderator

    Joined:
    Apr 11, 2010
    Posts:
    25,589
    Thank you so much! I would love more information please!
     
  8. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    Well it's as above, a single option that'll be on by default that causes us to reuse a single Collision/Collision2D instance for all collision callbacks. We acquire a strong lock on the instance to keep the GCs grubby-paws off it and release that when we shutdown and also when we do a domain reload too which happens when we exit play-mode but this is an editor-only concern.

    It results in no GC spew but as has been pointed out, there's the "contacts" property which is an array and again, we're back to leaving that to the GC. 2D solved this using some brute force techniques but they were never going to be a long-term solution. What we've decided to do here is to produce and acquire a strong-lock on an internal "List<ContactPoint>" and like the "Collision" instance, reuse it for each callback. We can manage its memory allocation but will likely start at a reasonable size anyway. It'll also mean that you'll need to grab your contacts during the callback (no references) but this is the same as the "Collision" instance anyway as it's the same instance reused. This behaviour will also only apply when the above option is on anyway.

    We'll likely change the "contacts" property to just do a "List<ContactPoint>().ToArray()" for backwards compatiblity however we can add various non-allocating "GetContacts()" methods.

    NOTE: We may not use a "List<ContactPoint>" at all but rather a NativeArray but it'll be hidden anyway and of no concern. What matters is no allocations of course.
     
    JustAnotherDude likes this.
  9. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    spajus and SugoiDev like this.
  10. herb_nice

    herb_nice

    Joined:
    May 4, 2017
    Posts:
    152
    still exhaling...
     
  11. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    > Absolutely, this isn't some far-off research or anything crazy like that. As far as I'm concerned, they'll be in the Unity 5 release.
    That work was going to be based on the new fast callback system (that part wasn't being developed by us) that never landed so our hands were tied. This work, based upon the new bindings system internal to Unity, untied them. ;)

    Unlike then, this work is actually done and running locally here. Hell, I've even got the docs done. Just got to write a bunch of tests and let QA have at it then onward to the release stream.

    In no small part is this work related to our conversation earlier sir. Bumped its priority!
     
  12. Partel-Lang

    Partel-Lang

    Joined:
    Jan 2, 2013
    Posts:
    1,937
    That's excellent news, been waiting for this for years! :D

    But to speed things up further, I'm thinking there are so many use cases for OnCollisionEnter/Stay that never need the contacts at all. Some just need relativeVelocity or impulse, so maybe there could be a "lite" version of the Collision class without the contact array?

    So just like we can have OnCollisionEnter without the Collision parameter, could we have another OnCollisionEnter with that contactless Collision parameter? Would that save us from the performance cost of filling in the contacts in the first place?
     
  13. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    Unfortunately that isn't an overload of the callback, it's just Unity playing nice and throwing away the "Collision" or "Collision2D" instance.

    The only realistic way to stop contacts being produced would be to have a global option that controls not populating contacts which isn't nice. Adding extra overloads of OnCollisionEnter,Stay & Exit isn't really a good option and would confuse a lot of devs. The next step is creating a completely new callback that doesn't suffer from these problems but we've solve that now so is kind of pointless.

    A better way would be a callback that simply indicates a contact for either collision or trigger and you retrieve what you like. This is another body of work however.

    In the end, the actual overhead of copying contacts though is very low, especially now because we do not create a new contacts array each callback and no special marshalling needs to take place.

    Good news is that I've finished the work and I have a pull-request in process for review. It's has passed physics review and is waiting on QA test/review as well as doc review then it's onward to 2019.1
     
    Partel-Lang likes this.
  14. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    The internal pull-request has just been approved after a full QA test so it is now beyond the event horizon and is falling inwards towards the 2019.1 singularity. Expecting it to land early next week.

    I have no idea if the release managers would approve this as a backport although it's more than possible to do that. I'll investigate this for 2018.3. I doubt it'd be allowed in the beta this late in the game but maybe in a patch release later.
     
    SugoiDev likes this.
  15. SugoiDev

    SugoiDev

    Joined:
    Mar 27, 2013
    Posts:
    243
    Having this on 2018.3 would be very nice! Thanks for the hard work.
     
    MelvMay likes this.
  16. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    This just landed in 2019.1 this morning. I've got a meeting with the release managers on Monday to do a risk analysis of landing this in late 2018.3 beta. I'll let you know what is decided.
     
    Cynicat, spajus, blurdot and 2 others like this.
  17. blurdot

    blurdot

    Joined:
    Jan 26, 2014
    Posts:
    20
    @MelvMay Wow, didn't see this since I was reading a different thread, but thanks for your hard work, this is great and can't wait to have it in 2019 and hopefully 2018.3 beta as well :D
     
    Last edited: Oct 30, 2018
  18. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    We have provisional approvement to backport this to 2018.3 beta. The PR is done and it's going through review and will have some extra QA shortly. Note that for this to land in 2018.3 beta we had to make a sensible consession which was to have the option off by default. We also don't have time to update all the templates (those would turn the option on in new projects) although this will be done for 2019.1. To turn this option on however will be a simple check-box.
     
  19. SugoiDev

    SugoiDev

    Joined:
    Mar 27, 2013
    Posts:
    243
    Oh man, that's some great news there. Can't wait to have a go at it. Awesome work!
     
  20. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    The backport for this no-gc work has just landed in 2018.3.0b11
     
  21. exzizt

    exzizt

    Joined:
    Sep 30, 2017
    Posts:
    65
    When can I get 2018.3.0b11? The hub just has 2018.3.0b9 at the moment.
     
  22. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    It hasn't been released yet.
     
  23. exzizt

    exzizt

    Joined:
    Sep 30, 2017
    Posts:
    65
    Any idea when?
     
  24. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    Looking at the schedule, 2018.3.0b10 has been published but not made public yet but I believe that should be in the next few days. 2018.3.0b11 is scheduled for publish on 16th Nov so a few days after that I guess.

    We're also almost on top of the release candidate so the next release after b11 might be b12 or an RC, not sure.
     
  25. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    You can try this feature right now but it's in 2019.1.0a8 which you can also download here but obviously it's an alpha so not necessarily in a great state, certainly not for production.
     
  26. exzizt

    exzizt

    Joined:
    Sep 30, 2017
    Posts:
    65
    Awesome! I'm excited to check it out soon.

    I downloaded 2019.1.0a8 and it's apparently been "upgrading" my project (made in 2018.2) for the past 20 minutes. I think something is broken? :(

    Any idea?
     
  27. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    No way I could know what is going on, sorry. You'll need to discuss any alpha issues in the 2019.1 alpha forum.
     
  28. Sabrino

    Sabrino

    Joined:
    Aug 8, 2015
    Posts:
    21
    Just wanted to add a thank you for this option, it was really necessary and it's a good solution.
     
    MelvMay likes this.
  29. exzizt

    exzizt

    Joined:
    Sep 30, 2017
    Posts:
    65
    Are we still getting 2018.3.0b11 on November 16th? Can't wait to try this out!
     
  30. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    Unfortunately I don't know exact dates when the releases will land as all I can see are the publish dates on an internal schedule page i.e. the date the build is built. It then has to pass QA verification etc. I certainly would expect it in November.
     
    exzizt likes this.
  31. KospY

    KospY

    Joined:
    May 12, 2014
    Posts:
    80
    Great news!
    This whole thing was killing my GC on my project.

    While we are talking about OnCollisionXXX methods, does it possible to add a way to retrieve the source collider from OnCollisionExit? OnCollisionExit do not send any contacts so I can't use collision.contacts[0].thisCollider.

    Thanks!!
     
  32. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    Because it has a behavioural change that some users would not want i.e. reusing the Collision/Collision2D instance for each callback. This would break existing projects that keep the instance beyond the callback and we do not want to break existing behaviour by just using a new version. New projects will have it on by default and it's a simple matter to turn it on yourself for existing ones.
     
    Last edited: Nov 16, 2018
  33. herb_nice

    herb_nice

    Joined:
    May 4, 2017
    Posts:
    152
    because we are suckers for punishment over here, we gave 2018.3.0b12 a quick spin. we did NOT test this build thoroughly... however, after checking the recycling box in physics 2d settings, it appears that collisions are no longer spewing garbage all over the place. this looks promising.

    also, it looks like RenderPipelineManager.CleanupRenderPipeline() is no longer spewing 80 bytes of garbage every frame. this is also good.

    however, a new thing is spewing 17 bytes of garbage every frame. ScriptableRuntimeReflectionSystemWrapper is the new disappointment. The dreams of a garbage free build are closer but still so far away...

    upload_2018-11-30_13-12-43.png
     
  34. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    I spotted this whilst getting my build gc alloc free and reported it to the appropriate team. A bug case was reported already so hopefully they'll sort it out. Apparently it's due to some function returning bool which causes boxing in some wrapper ... blah blah blah. Anyway, bug reported, hopefully sorted soon.
    Such a dark view of the future! Fear not!
     
    spajus and herb_nice like this.
  35. MostHated

    MostHated

    Joined:
    Nov 29, 2015
    Posts:
    851
  36. herb_nice

    herb_nice

    Joined:
    May 4, 2017
    Posts:
    152
    gave 2018.3.0f2 a spin. editor locks up every other time that you press play, and that 17 bytes of scriptableruntimewhatever still leaks every frame.

    HOWEVER... https://docs.unity3d.com/2018.3/Documentation/ScriptReference/Scripting.GarbageCollector.GCMode.html <- this appears to work.

    so now we disable the garbage collector after everything is instanced, and re-enable it and manually call gc.collect on boss deaths, player deaths, screen transitions- anywhere that it is ok to drop a frame. and then disable it again immediately. seemed good on initial tests, more in-depth test is building now.
     
  37. herb_nice

    herb_nice

    Joined:
    May 4, 2017
    Posts:
    152
    tests went well, game ran very smooth.

    due to potential out-of-memory paranoia we added a gc.collect call if gc.collect has not been called for 32768 frames, roughly 9 minutes at 60hz, as by then more than half a meg of garbage will have piled up from that scriptableruntimewhatever.
     
    MostHated and MelvMay like this.
  38. kersk

    kersk

    Joined:
    Jan 2, 2012
    Posts:
    56
    @MelvMay Is there any news on that team fixing ScriptableRuntimeReflectionSystemWrapper.cs? This is still occurring in 2018.3.4f1 and it's the only thing allocating per frame in my project. Please hound them to fix it! :)
     
  39. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
    I pinged the person who's got this on their TODO again but I'm not the only one doing the same. Hopefully it'll land soon.
     
  40. Barkers-Crest

    Barkers-Crest

    Joined:
    Jun 19, 2013
    Posts:
    154
    @MelvMay Is there a link to the bug report for ScriptableRuntimeReflectionSystemWrapper.cs?
     
  41. MelvMay

    MelvMay

    Unity Technologies

    Joined:
    May 24, 2013
    Posts:
    1,961
  42. herb_nice

    herb_nice

    Joined:
    May 4, 2017
    Posts:
    152
    ^ the ScriptableRuntimeWhatever 17 bytes of garbage spew appears to finally be corrected in 2018.3.8f1
     
    MostHated and Barkers-Crest like this.