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

chore: Bullet 2.89 #322

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

Conversation

ianpurvis
Copy link
Collaborator

Updates to bullet 2.89 (https://github.com/bulletphysics/bullet3/releases/tag/2.89)

Excluded:

  • bullet/data
  • bullet/doc
  • bullet/examples
  • bullet/test

Ammo patches:

  • bullet/src/BulletDynamics/Dynamics/btDiscreteDynamicsWorld.h
  • bullet/src/BulletDynamics/Dynamics/btRigidBody.h

@ianpurvis
Copy link
Collaborator Author

@kripken I wanted to preserve some flexibility in the repo history, so I walked all of bullet's tagged releases and squashed them into individual commits. So we just have to take care about re-squashing to master if we want to preserve that.

Unfortunately, bullet's source tree is really bloated these days... 329MB with all files included. To keep ammo peppy, I excluded a number of directories and was able to shrink it down to 15MB. But if you think it's good to include some or all of those sources, let me know. I have some intermediate branches and can restore them pretty easily.

Other than that, let me know if you like this approach 👍

@kripken
Copy link
Owner

kripken commented Aug 3, 2020

Sounds good in general!

How does this affect code size and performance?

@ianpurvis
Copy link
Collaborator Author

Sorry for the delay, I wanted to spend some time with this...

Code size has grown a little, for example the total wasm footprint grows by 44K to 340K.

Performance unfortunately did not improve. The WebGL demo runs roughly the same on my laptop, however the stress test is not performing so well. The wasm stress test avg time increased by 136% 🤔

Maybe there is a newer bullet build configuration that we could use to improve things. I'll see what I can find.

I built everything on macOS w/ emsdk 1.39.20

Size

master (aa26a62)
$ du -h builds/ammo.*
1.7M	builds/ammo.js
388K	builds/ammo.wasm.js
640K	builds/ammo.wasm.wasm

$ gzip -k builds/ammo.*
$ du -h builds/ammo.*.gz
392K    builds/ammo.js.gz
 48K    builds/ammo.wasm.js.gz
248K    builds/ammo.wasm.wasm.gz
chore/bullet-2.89
$ du -h builds/ammo.*
1.9M	builds/ammo.js
388K	builds/ammo.wasm.js
768K	builds/ammo.wasm.wasm

$ gzip builds/ammo.*
$ du -h builds/ammo.*.gz
448K	builds/ammo.js.gz
 48K	builds/ammo.wasm.js.gz
292K	builds/ammo.wasm.wasm.gz

WebGL Demo, 1000 cubes

macOS Chrome Version 84.0.4147.105 (Official Build) (64-bit)

master (aa26a62)
examples/webgl_demo/ammo.wasm.html  ~44 fps (stable)  
examples/webgl_demo/ammo.html       ~20 fps (stable)
chore/bullet-2.89
examples/webgl_demo/ammo.wasm.html  ~43 fps (stable)  
examples/webgl_demo/ammo.html       ~18 fps (stable)

Stress Test

master (aa26a62)
$ AMMO_PATH=builds/ammo.wasm.js ./profile 10
2.21890000000000000000
$ AMMO_PATH=builds/ammo.js ./profile 10
3.52330000000000000000
chore/bullet-2.89
$ AMMO_PATH=builds/ammo.wasm.js ./profile 10
5.23810000000000000000

$ AMMO_PATH=builds/ammo.js ./profile 10
10.19750000000000000000

☝️ profile is just a cheap script to average run times from stress test log:

#!/usr/bin/env bash
# profile <iterations>
iterations=${1:-1}
for i in $(seq 1 ${iterations}); do
  npx ava --verbose --no-cache tests/stress.js
done \
  | sed -n -E 's/.*total time: (.*)/\1/p' \
  | paste -s -d+ - \
  | echo \($(cat)\)/${iterations} \
  | bc -l

@willeastcott
Copy link
Contributor

Does native Bullet exhibit similar perf/size differences between 2.83 and 2.89? These numbers make me very nervous about rolling out 2.89 to PlayCanvas developers.

@ianpurvis
Copy link
Collaborator Author

@willeastcott I was surprised when I finally checked the numbers. Testing native bullet is good idea. Do you know of a good perf test for bullet, something that we could run between 2.83 and 2.89 ? In a perfect world, it would be nice to port that to javascript and run the same test on the ammo side. Alternatively I guess we could try to port the ammo stress test to native bullet and look for some indicators. I'm not a bullet expert, so I will definitely take advice.

Here are the size differences that I see for the bullet libraries built with ammo.

chore/bullet-2.89
$ find builds-2.89/ -name '*.a' -execdir du -h {} \;
564K	libBulletSoftBody.a
1.0M	libBulletCollision.a
800K	libBulletDynamics.a
108K	libLinearMath.a
master (aa26a62)
$ find builds-master/ -name '*.a' -execdir du -h {} \;
244K	libBulletSoftBody.a
924K	libBulletCollision.a
492K	libBulletDynamics.a
100K	libLinearMath.a

@ianpurvis
Copy link
Collaborator Author

Btw, I found these benchmark ideas from an old thread - #117 (comment)

  1. A stack of a large number of spheres - bottom one is static - the engines should have no problem maintaining a stable stack. The sphere-sphere collision detection is fast and unlikely any engine has a poor implementation, this would test the engine's ability to create/manage memory, update/integrate objects, and resolve constraints (contacts & friction).
  2. Same as above but multiple stacks, don't need as many spheres per stack. This is to test how the engines do with parallel constraint solving via islands (Goblin doesn't do this yet, don't know if Oimo does or not, should see noticeable improvement in Cannon & Ammo).
  3. Constraints. Lots of them. Some combination of hinges and point-to-point/socket constaints would do the trick (present in all four engines, easy to setup, representative of everything the engine needs to run for constraints).
  4. Ray casting. Doesn't look like Oimo supports this. I expect Ammo will destroy Cannon and Goblin here as neither have a bounding-volume-hierarchy broadphase, only SAP which is terrible at ray casting performance.
  5. Convex hull - collision. Complex geometry can be generated by a convex hull from a list of vertices - the minimum convex shape required to wrap all of the vertices. Oimo doesn't support this; Cannon can't generate this shape from a list of vertices but if you have the vertices & faces it can construct the shape.
  6. Convex hull - creation. only Ammo and Goblin have this.
  7. Compound shape collision, used to combine multiple simple shapes into a complex one. Appears only Goblin and Ammo have this.
  8. Concave-Convex collision. Not supported by Oimo. Cannon has TriMesh, Goblin has MeshShape, and Ammo has btConcaveShape (well, Bullet does). These can all be collided with convex geometry.
  9. Concave-Concave collision. Not supported by Oimo, I don't know if Cannon's TriMesh can collide with other TriMeshes. Bullet's btConcaveShape must be static and won't move, but Bullet has GImpact classes that allow for dynamic concave-concave collision. Goblin's MeshShape can collide with other MeshShapes
  10. Bounding volume hierarchy creation. I don't know how much the various engines expose so this may be difficult. Ammo, Goblin, and Oimo are able to construct an AABB BVH tree.

@kripken
Copy link
Owner

kripken commented Aug 7, 2020

Bullet comes with a Benchmark Demo, which for example emscripten runs in its benchmark suite,

https://github.com/emscripten-core/emscripten/tree/master/tests/third_party/bullet/Demos/Benchmarks

@ianpurvis
Copy link
Collaborator Author

Thanks! I can push it forward with that.

@ianpurvis ianpurvis mentioned this pull request Sep 17, 2020
@Galadirith
Copy link

Galadirith commented Oct 11, 2020

I've been looking at updating Ammo to use 2.89 in n5ro/aframe-physics-system#171, I really apologise I didn't check if this was already only going work. I have been looking into the performance degradation with [email protected] and I wanted to share my findings, which I hope are useful and not just adding noise.

I found that 2.85 to 2.86 bulletphysics/bullet3@2.85...2.86 gave the largest drop in performance. I managed to bisect my way through the commits and found that bulletphysics/bullet3@dcd02a1 looks to be the commit that introduces the largest performance drop, at least in the stress test. I wouldn't recommend actually building against dcd02a1 because it has very verbose output, and instead use 8ff1e55 which is just a small commit that simply disables the verbose output. Here are some profile results from the two commits before and two commits after dcd02a1

$ AMMO_PATH=builds-75633d5/ammo.wasm.js ./profile 32
2.77909375000000000000

$ AMMO_PATH=builds-38b1013/ammo.wasm.js ./profile 32
2.79343750000000000000

$ AMMO_PATH=builds-8ff1e55/ammo.wasm.js ./profile 32
6.28125000000000000000

$ AMMO_PATH=builds-379a852/ammo.wasm.js ./profile 32
6.29387500000000000000

It looks like bulletphysics/bullet3@dcd02a1 introduced quite a few simple arithmetic operations and a comparison at each iteration of the solver.

I still see something like 10-15% performance degradation in the stress test going from bulletphysics/bullet3@379a852 to bulletphysics/bullet3@830f0a9 (2.89), but this performance degradation before 2.86 is by far the largest, more than halving the performance of the stress tests.

I completely appreciate that @ianpurvis is working on much more detailed and complete benchmarks in #336, but I hope this is useful towards resolving performance issues. I did attempt to simply revert bulletphysics/bullet3@dcd02a1, but the function names and interfaces have changed enough that this isn't trivial. But it does look doable and if it could be done, may resolve most of the performance degradation in 2.89, and possibly eliminate any perceived real world performance degradation 😊

@ianpurvis
Copy link
Collaborator Author

@Galadirith This is rad! Thanks for sharing your info, definitely appreciated. I'll take a look at bulletphysics/bullet3@dcd02a1 and see if I can figure out a path forward. If we can fix this in ammo, it would be great to submit a patch to upstream bullet as well.

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