4

I am getting an "Expensive Method Invocation" AND a "Null comparison expensive" warnings and I would like to know how to fix these.

    void Update()
    {       
        CheckCollision();
    }

and in CheckCollison where the errors are happening, shown in comments below,.

    void CheckCollision()
    {
        var topHit = Physics2D.OverlapCircle(topCollision.position, 0.2f, playerLayer);

        if (topHit != null) // ***<<<< NULL COMPARISON EXPENIVE!***
        {
            if (topHit.gameObject.CompareTag("Player"))
            {
                if (!stunned)
                {
                    var rigidBodyTopHit = topHit.gameObject
                    .GetComponent<Rigidbody2D>();  // ***<<<< EXPENSIVE METHOD INVOCATION!***

                    rigidBodyTopHit.velocity = new Vector2(rigidBodyTopHit.velocity.x, 7f);
                    canMove = false;
                    _myBody.velocity = new Vector2(0,0);
                    _animator.Play("SnailStunned");
                    stunned = true;
                }
            }
        }
        if (!Physics2D.Raycast(downCollision.position, Vector2.down, 0.1f))
        {
            ChangeDirection();
        }
    }
Mark Smith
  • 437
  • 1
  • 6
  • 11
  • 2
    These are not warnings, and in your case, can't be "fixed" as you want to get something that is dynamic. Rider simply gives you hints on best practices. You can alt+enter on any Rider suggestion -> Why is Rider suggesting this ? Unity just released a video about Rider, you should check it out https://www.youtube.com/watch?v=2CvSoo0hlWI – Jichael Sep 09 '19 at 07:30
  • The expensive probably refers to `GetComponent` which in your case can't be avoided really .. you might want to store the last reference and try to re-use it instead of all the time using `GetComponent` .. but that's only a very small performance fix. The second one you can use `if(topHit)` since all components and GameObjects implement a [`bool` operator](https://docs.unity3d.com/ScriptReference/Object-operator_Object.html) which is slightly more efficient then a `!= null` – derHugo Sep 09 '19 at 15:51
  • In general everything related to Physics and Physics2D should rather be done in `FixedUpdate` instead of every frame in `Update` – derHugo Sep 09 '19 at 15:53

1 Answers1

17

These are not warnings, but informational highlights. They are intended to inform you that you are doing something that is known to be expensive, inside a performance sensitive context (the Update method) and you can re-evaluate your approach if necessary.

The "if necessary" bit is important - only you can decide if the performance characteristics of this Update method are appropriate for your game/application. If this happens in a single game object in an options screen that's rarely used, then it's most likely not a problem. But if it's in an object that is instantiated many times and used in core gameplay, maybe these expensive methods will add up and reduce performance (or affect battery life).

Generally speaking, it's probably going to be very hard (and unnecessary) to write code that does not have any of these informational highlights, but if you find that you have a lot of code that has a lot of highlights, then it's a good indication that you should profile and see if you're happy with the results.

You can find out more about these highlights by using Alt+Enter, expanding the inspection options item and selecting the "Why is Rider/ReSharper suggesting this?" item. This will open a web page with more details about that particular highlight.

This page also has an overview and more details, and the inspections can also be disabled in the Preferences | Editor | Inspection Settings | Inspection Severity | C# settings page (under Unity | Performance indicators).

Looking at the items marked in the code sample, the equality comparison is expensive because it also checks to see if the underlying engine object has been destroyed, which means a transition into native code. If you know that the object hasn't been destroyed (and seeing that it's returned from a Unity API, I think that's a safe bet), then you can replace it with:

if (!Object.ReferenceEquals(topHit, null)) {
  // … 
}

But remember that this is a micro-optimisation that is potentially unnecessary, in your use case.

(Calling the implicit bool operator with if (!topHit) is exactly the same as if (topHit != null), both operators call the private Object.CompareBaseObjects method)

The second item is a call to GetComponent, which is known to be expensive, but is necessary in this context. That's ok - this is just informational, not telling you that you've done something wrong.

citizenmatt
  • 18,085
  • 5
  • 55
  • 60