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

emscripten build of the benchmark itself, not through box2d.js #5

Merged
merged 1 commit into from Feb 27, 2012

Conversation

kripken
Copy link
Contributor

@kripken kripken commented Feb 27, 2012

This benchmark build is half as big, and a bit faster.

@nathanhammond
Copy link
Contributor

Can we leave test.js/the existing test as is? I think having both approaches (emscripten built benchmark, JS binding benchmark) are both valuable pieces of information.

But in any case, very impressive that emscripten build is faster than the hand-written JS code. Sometime later today I'm going to run the JS version through a profiler to see if there are any hotspots for quick performance wins.

@kripken
Copy link
Contributor Author

kripken commented Feb 27, 2012

My only concern with leaving them as is is that people might not be sure which is the right one (for purposes of bench2d). How about just linking to the box2d.js files?

In general compiled code can be faster than handwritten code, the compiled code avoids creating objects, garbage collection, etc.

@joelgwebber joelgwebber merged commit 9414bd2 into joelgwebber:master Feb 27, 2012
@nathanhammond
Copy link
Contributor

Alon, I think you might want to have the "default" benchmark be the box2d.js linked version instead of the emscripten transpiled version of the benchmark:
https://docs.google.com/spreadsheet/ccc?key=0ApnSRONV3LTVdDIwSDNiQ0VQb2tHQnM4N1V0NEc5Z1E

@kripken
Copy link
Contributor Author

kripken commented Feb 27, 2012

Hmm, that's odd. For one thing, the difference should be very small. For another, it should be the other way.

What might be happening is that the C++ compiled version has more inlining and hence larger functions. FF12+ does much better than FF10 on that, because FF10 recompiles too often.

@joelgwebber , is it possible to have both versions of the benchmark?

edit: forgot to say, thanks @nathanhammond for the data!

@kripken
Copy link
Contributor Author

kripken commented Feb 27, 2012

@nathanhammond , on Firefox Nightly and Chrome Dev I see no speed difference between the two versions. So it could just be an issue in earlier browsers. But it's surprising they would be coordinated that way. Was your benchmarking here with the files from @joelgwebber 's repo, or with the more-optimized test.js in box.js?

@nathanhammond
Copy link
Contributor

I'll create a pull request later this evening that encompasses both of them, probably setting up kripken/box2d.js as a submodule so that updates are automatic.

@nathanhammond
Copy link
Contributor

@kripken The result in the spreadsheet I posted is from my update to test.js in box2d.js, however, I was seeing similar but less pronounced results prior to my modifications (the JS benchmark still performing better overall than the C++ one).

The C++ version I tested against is the one you committed from earlier today: bench2d.js.

I triple-checked to make sure I was indeed entering the results for the test in the correct field and did browser restarts between runs for memory eviction.

I'm going to load up the nightly browsers in a second and see what those look like on my box.

@kripken
Copy link
Contributor Author

kripken commented Feb 28, 2012

I tested on FF 10 over here meanwhile, and I see no significant difference in the box2d vs C++ benchmark versions (with up to 0.5% of noise). So my suspicion is that the main factor here is your optimizations to test.js, apparently they help significantly!

@joelgwebber : I wonder if those optimizations are acceptable under the rules of your benchmark? The main loop is tweaked to help JS engines warm up more effectively, but then the main loop is no longer identical to the other implementations. Or is it ok to tweak it that way, if the box2d processing remains identical (which it does)?

@nathanhammond
Copy link
Contributor

@kripken @joelgwebber For my 2¢, I don't think it's unfair unless we're going to include VM warmup time in all of the tests. By splitting those two loops you're forcing the JS VMs to recreate their much higher-performance loop tracing during the test. (In light of that, maybe the other benchmarks are broken!)

Also:

  • In any "real-life" use-case for box2d everything will be running in a single loop.
  • This is a lot different from hand-tweaking a transpiled result, this is just optimizing the way in which you interface with the underlying box2d library, in whatever language you're in.

@joelgwebber
Copy link
Owner

Just to make sure I understand the optimizations in question -- in the linked fork, I see two implementations, emscripten-js and emscripten-c. The -js version appears to use the js-friendly bindings, while I presume the other is compiled into a single executable using the C++ test source. I don't actually see the C++ source for the test anywhere, but I presume it's roughly the same as the one in /c.

I see no obvious problem with testing this way. My main goal with these benchmarks is to get a sense of the relative performance of any practical mechanism for implementing a game using Box2D that runs more-or-less correctly. I see no reason to object to either approach, as long as the output produces sane values. I presume there are no other "cheating" issues here that I'm not seeing?

One nit on the form of the patch itself. If we can determine that one of these approaches is generally faster than the other, or that it's a wash, I have a slight preference for keeping just one in the repo. And if it's the C++ version, I'd at least like to have the source in there for comparison (not too concerned with whether it builds, though). Finally, if we need to keep both versions, would you mind reorganizing them under a single root (e.g., /emscripten/c, /emscripten/js)? Thanks!

@nathanhammond
Copy link
Contributor

You're correct in all things. I didn't want to get too far ahead of myself so I went ahead and "showed my work" if you will (don't feel pressured to accept that pull request yet). I'm currently working on incorporating the build mechanisms for each approach so that benchmark setup is as simple as "git clone bench2d; git submodule update --init; make;"

@joelgwebber
Copy link
Owner

Cool, SGTM then. Let me know when you're ready and I'll pull it in. And with any luck I'll have the Flash stuff straightened out (it's remarkably painful to get the logs out of Flash...).

@kripken
Copy link
Contributor Author

kripken commented Feb 28, 2012

@nathanhammond I reverse my position from before then, if it's simpler to just have one version, sounds like your optimized one with box2d.js is the right choice.

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.

None yet

3 participants