Fix loop performance

[David-Beale]

1.3.1 introduced some small performance issues. This gets everything working perfectly again for me

[drcmda]

can you measure these performance regressions? it should actually be faster because it's not using two raf's but one, this pr would go back to two. i did measure this and useFrame was considerably faster. 🤷‍♂️

2 raf

1 useframe (current)

[David-Beale]

I'm wondering if the logic is out of sync by a frame. It wouldn't be noticeable to the naked eye, but my AI has been trained using cannon-js, and worked perfectly before 1.3.1. Afterwards, it's unable to make certain moves. I assumed it was lag, but could also be because of a 1 frame delay.

[David-Beale]

I'm not very familiar with useFrame, but my theory is that it posts a "step" message 60 times a second regardless of any "backlog" in the cannon-js thread. So I'm guessing it's likely that it's sending another step message before the buffer positions/quaternions have been updated.

In previous versions, the next step message can only ever be sent once the frame message is received and the positions/quaternions have been updated.

[David-Beale]

Comparison of the two:

https://www.youtube.com/watch?v=MxmzP7VsMYU

Originally it reacts at the perfect moment to barely make the turn.

Now it seems to be reacting a fraction of a second late.

[drcmda]

useframe binds the component to the global render-loop. if a component needs to execute something in a loop it usually should be done in there in order to not cause raf requests. but what you say seems valid. i just wonder where these huge raf/onmessage blobs come from if we switch to the old version, they seem pretty threatening to me.

the problem is that we're dealing with two things here, one is the provider with its loop, the other is the useBody hook which is in all physics components.

hooks.tsx:

  // Run loop *before* the main loop in the provider!
  useFrame(() => {
    if (ref.current && buffers.positions.length && buffers.quaternions.length) {
      if (ref.current instanceof THREE.InstancedMesh) {
        for (let i = 0; i < ref.current.count; i++) {
          const index = bodies.current[`${ref.current.uuid}/${i}`]
          if (index !== undefined) {
            apply(temp, index, buffers)
            temp.updateMatrix()
            ref.current.setMatrixAt(i, temp.matrix)
          }
          ref.current.instanceMatrix.needsUpdate = true
        }
      } else apply(ref.current, bodies.current[ref.current.uuid], buffers)
    }
  }, -2)

provider.tsx

worker.onmessage = (e: IncomingWorkerMessage) => {
      switch (e.data.op) {
        case 'frame':
          buffers.positions = e.data.positions
          buffers.quaternions = e.data.quaternions

...

  const loop = useCallback(() => {
    if (buffers.positions.byteLength !== 0 && buffers.quaternions.byteLength !== 0) {
      worker.postMessage({ op: 'step', ...buffers }, [buffers.positions.buffer, buffers.quaternions.buffer])
    }
  }, [])

  // Run loop *after* all the physics objects have ran theirs!
  // Otherwise the buffers will be invalidated by the browser
  useFrame(() => loop(), -1)

in order for this to work we get the onmessage, now we need to distribute data to the bodies, then buffers can be sent back. if they're sent back before all bodies have been updated we have a problem because buffers would be invalid.

perhaps the best thing to do would be to immediately write the data to the bodies and then just do the post. it's good to pluck this apart again, this is the most critical part of the lib and caused me endless headaches until i finally read up on the shared buffers spec.

[drcmda]

redoing that part rn, it can copy over positions inside onmessage

[David-Beale]

Interestingly, simply changing the loop render priority from -1 to Infinity solves my problem. Would that mess anything up?

[drcmda]

Yes, but only work if you work with fixed models, if you are adding something at runtime it should be stuck. But I have it working, I test a little more and then push.

[David-Beale]

awesome, thanks

[drcmda]

btw @David-Beale your game looks interesting! maybe think about joining https://github.com/pmndrs/racing-game

[David-Beale]

btw @David-Beale your game looks interesting! maybe think about joining https://github.com/pmndrs/racing-game

Thanks. Yeah I'll definitely have a look once I'm finished with this project.

[David-Beale]

I've figured out a work around for my problem. I just need to make sure my neural network activates before the loop function. So setting my component render priority to -2 fixes everything without having to edit the library.

[David-Beale]

Ignore all the nonsense about "backlogs". It's just a timing issue. All component interactions seem to be delayed until the next frame unless manually adjusting the render priority.

I feel like setting the loop render priority to Infinity should work. I'm still able to add and remove objects on the fly without any problems

[David-Beale]

Could also be because the subscriptions are out of sync. Sorry, lots of guess work.

[drcmda]

i've updated the lib. now it writes to the objects inside onmessage and all this framing back and forth goes away. i also use matrixautoupdate=false and matrix instead of position and rotation by quaternions, that should also be a little faster.

[David-Beale]

Thanks. Doesn't solve my problem though, I still need a manual render priority < 0 :pensive: .

Also introduced a breaking change: The position and quaternion in the mesh object are no longer updated:

I need to switch to getWorldPosition() or a subscription, but perhaps I should have been doing that anyway.

[drcmda]

that's odd, i mean, what better place than update objects in onmessage instead of a frame late. the position/quat may be collateral, but i think it should probably have been matrix all along because it's faster and nothing else but physics can place these objects.

[David-Beale]

I think what's happening is:

values1 => step => apply forces(values1) => update values(values 2) => step => apply forces(values2)

whereas I need it to be:

values1 => apply forces(values1) => step => update values(values 2) =>apply forces(values2) => step

I admit it makes no difference in 99.9% of cases so I'm happy to use manual render priority as a fix.

re position/quat. I was just using them for my camera target, so makes sense to switch to getWorldPosition() anyway.

I really appreciate your time and help with this :pray: . Happy to close this PR.