Improve Performance of Mutating Blocks

[BeksOmega]

Describe the bug

Current performance on 7eebd78:

Time to execute updateWorkspace = ~188 ms averaged over 5 trials.

Convert the following txt file to a json file and then load it into your Chrome Dev Tools to view the results:
mutating-1.txt

To Reproduce

Steps to reproduce the behavior:

1. Go to the playground.
2. Load the following JSON:

{
  "blocks": {
    "languageVersion": 0,
    "blocks": [
      {
        "type": "controls_if",
        "extraState": {
          "elseIfCount": 3
        },
        "inputs": {
          "IF0": {
            "block": {
              "type": "logic_boolean"
            }
          },
          "IF1": {
            "block": {
              "type": "logic_boolean"
            }
          },
          "IF2": {
            "block": {
              "type": "logic_boolean"
            }
          },
          "IF3": {
            "block": {
              "type": "logic_boolean"
            }
          }
        }
      }
    ]
  }
}

4. Go to the performance tab of the chrome devtools.
5. Enable recording screenshots.
6. Set CPU Throttling to 6x.
7. Start a recording.
8. Open the if-block's mutator.
9. Add a new else-if at the top.
10. Stop the recording.

Expected behavior

Ideally, we wouldn't drop frames when mutating blocks. For this to be achieved, mutating needs to complete within 10ms. Honestly don't think this is possible, but it's something to shoot for.

Additional context

Some ideas:

• No clue why we have so many duplicate compose calls. I'm hoping to get rid of those.
• I also want to remove the forced layouts.
• I might also try to delay resizing the mutator bubble using a setTimeout since that's not critical.

Also note that the plus-minus version of the controls-if is ~4x faster. Honestly I expect it to be even faster than that since it doesn't have duplicate compose calls. So I might try to dig into that as well. (really hoping it's not caused by compilation)

[ewpatton]

@BeksOmega Saw this come across my desk and it reminded me of mutator performance issues we had in App Inventor. One thing you might want to do is see if you could memoize the mutator workspace state and compare it on every event to determine whether to call compose. The second thing is the insertion marker since it behaves somewhat like a block. I wonder if the appearance of the insertion marker is causing the extra compose calls you observed (which memoization should in theory address since the insertion marker isn't part of the workspace state, I think). Third, and maybe separately, because of this some behavior is breaking: I followed your instructions above, and then used the Alt-drag technique to pull the newly inserted else-if out of the stack and all of the test values were disconnected and bumped. This may be a different issue entirely though as it appears the mutator stack isn't healed until the block is dropped and so updateShape_ ends up with elseifCount_ = 0 in the interim destroying the connections.

[BeksOmega]

the insertion marker since it behaves somewhat like a block. I wonder if the appearance of the insertion marker is causing the extra compose calls you observed

I think you're probably right. But it could also be drag events that don't actually connect/disconnect blocks. I need to do some more logging to find out.

which memoization should in theory address since the insertion marker isn't part of the workspace state, I think

They're not supposed to be part of the workspace state, but because insertion markers are just implemented as BlockSvgs with an isInsertionMarker flag set to true, they often end up there. We have checks all over the codebase now to try to handle this. See #5375 for more info.

But in this case yes, we've removed them from the serialized state, so memoization should work =) I'll do some testing to see if memoization + diffing is faster, or if filtering the events is faster.

I followed your instructions above, and then used the Alt-drag technique to pull the newly inserted else-if out of the stack and all of the test values were disconnected and bumped.

Yikes. Yeah I think that's a separate issue from this performance one, but definitely something we want to fix. Reminds me of to #6033, so I wonder if it's also an insertion marker thing. I'll file a new issue to track that bug.

Thank you for the feedback Evan :D Really appreciate your thoughts! And for finding that bug =)

[BeksOmega]

I tried enabling the new render queueing for mutations, and it actually makes us drop more frames because the browser does an extra style recalc/layout in between composing and rendering.

Since we drastically improved performance, and we don't drop frames unless we're CPU throttling, I'm going to go ahead and close this as fixed.