Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optimization of Body's hidden class #528

Merged
merged 2 commits into from
Jun 12, 2018

Conversation

bchevalier
Copy link
Contributor

@bchevalier bchevalier commented Nov 24, 2017

I noticed that the number of properties on the bodies were not always the same. It varied between 45 and 48. Which is bad for performance as the JS compiler and optimizer cannot take advantage of consistent hidden classes when compiling JS code.
That is to say, having different number of properties per body meant that the optimizer could not optimally optimize some methods (apparently the most unoptimized methods were solverVelocity and Body.update).

Changes

  • Made sure all 48 properties were initialized at body creation time.
  • Bonus change: simplified the Body.set method as it was never called with a string as a 2nd argument.

Results

The gain is especially high on test using mixed shapes where more than 2 hidden structures had to be supported. The following benchmark was done on the Mixed Shape test:
Before
screen shot 2017-11-25 at 12 35 31 am

After
screen shot 2017-11-25 at 12 35 50 am

On a fairly similar profiling duration we can notice that the consumption of the engine is about 25% lower after the optimization!

Gains are usually not that high and seem to indicate a performance boost of 10% in most cases.

Note: Taking a memory snapshot both before and after seemed to indicate that the memory footprint increase was unnoticeable:
screen shot 2017-11-25 at 12 44 23 am

(Snapshot 4 is before and Snapshot 5 after)

@photonstorm
Copy link

Just tested this across several light and heavy tests. On average I'm seeing Engine.update go from 63.2% down to 51.8% with no significant memory difference.

Am seeing regular gc flushes though, causing a traditional peaked mountain effect. It did this before making this change too though. Resolver.solveVelocity remains the main CPU hog across all tests.

@bchevalier
Copy link
Contributor Author

I think it's possible to reduce object creation (by creating pool of reusable objects) although I haven't experienced it as being an important issue yet. What browser are you testing on?

@liabru
Copy link
Owner

liabru commented Feb 10, 2018

Looks great other than the change to the signature of Body.set. I know that internally the string option isn't used but it's intentionally there as a shorthand for users.

Can you revert that part so that feature remains? Thanks!

@bchevalier
Copy link
Contributor Author

Done!

@liabru liabru merged commit e60ebac into liabru:master Jun 12, 2018
@liabru liabru mentioned this pull request Dec 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants