Aglorithm optimization of Pairs.update method

[bchevalier]

Changes

Added a boolean on the Pair object that saves whether a pair has been confirmed as active at the current iteration.
The idea was to reduce the complexity of the second loop of the function from O(n*m) to O(n). Where n is the number of pairs and m the number of active pairs.

Result

Profiling on a specific test case indicated that the indexOf method was using about 13% of CPU resources used by the matterjs:

We can notice that the Pairs.update method is consuming 27% of the total.

After the optimization the profiling looks like this:

Notice that the indexOf method disappeared and the Pairs.update method is now consuming only 16% of the total.
Overall the engine is ~13% faster on that particular test case (about 50 bodies). The benefit should get bigger as the number of bodies in the world increase.

[payload]

looks good 👍

[photonstorm]

So glad you did this! I noticed today in a simulation with 1000 bodies that Common.indexOf was eating up a staggering 68% CPU time. I refactored it out to be a normal Array check and it helped a lot, but was still using masses more processing than you'd think. This fix looks even better. Will it be merged into master?

[photonstorm]

Here's the results of a test with 512 circular bodies added to the world before making the change in this PR (although I did factor out Common.indexOf):

And here is the same demo using the code in this PR:

It's night and day :)

[bchevalier]

@photonstorm , looking at your benchmark, w.r.t. some tests I made you could get an extra 10% boost without controversial changes to the engines. For instances, Resolver.solvePostion can be brought down to ~2%, Pair.update to almost nothing.

I imagine that the method _overlapAxes can probably be sped up by a factor of 2 when dealing with rigid bodies. And I think Pairs.update and Detector.collisions can be sped up too, not sure to what extent.

Is the source code of your demo available online? so I can benchmark future optimizations?

[photonstorm]

@bchevalier Sort of, the demo I was testing was this one, but you'll notice it's wrapped in Phaser 3 code. Even so, the actual matter.js parts of it are quite tiny really, so easy to re-create. You're welcome to just throw ideas at me and I'll implement locally and re-test.

http://labs.phaser.io/view.html?src=src\physics\matterjs\drag%20through%20balls.js

[bchevalier]

Cool, if you wish you can simply test my PRs, I just suggested an improvement of the Pair.update method: #526

[liabru]

Excellent work on these changes @bchevalier! It will take me some time to review them all since they are pretty core changes but things are looking good from my initial testing!

[bchevalier]

@liabru, thanks for having a look!
I noticed that matterjs maybe the most widely used/most active JS physics engine. I do not know much about physics engine but realized that it could benefit from some general optimizations.

[bchevalier]

Hey @liabru, I was wondering if you had any feedback on those PRs, any blocker?

[liabru]

Hey sorry for the delay on these guys, busy December (and I needed a little break).
I have an experimental branch with your first batch merged in and everything looks good.

I still need to take a look at your new batch. Thanks for all the great contributions!

[liabru]

I've had a closer look at this, there's an issue here. The collisionEnd event fires multiple times now, you can see this by adding these lines to one of the examples and ending a collision:

Matter.Events.on(engine, 'collisionEnd', function(event) {
  console.log(event.pairs.length, engine.timing.timestamp);
});

I think the fix is this on line 97:

if (pair.isActive && !pair.confirmedActive) {

Could you try this out and update the pull request? Otherwise I'll apply the fix. Thanks!

[bchevalier]

Hey, thanks for the feedback, I will probably be addressing all the issues in 2 weeks' time.

[bchevalier]

Fixed, seems to work properly now.

[photonstorm]

Woohoo! Merged at last :) 🕺