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

Fix compilation with MSVC14 and C++14 compilers, and gets rid of pthread-win32 #136

Merged
merged 6 commits into from Aug 25, 2017

Conversation

SirLynix
Copy link
Contributor

@SirLynix SirLynix commented Oct 13, 2016

Hey.

I'm looking at using Chipmunk as a 2D physics engine for my game engine and I had some troubles compiling it under Windows because of MSVC now supporting users litterals and because of the pthread.h include, so I did fix that.

I also reimplemented a very small pthread implementation based on Win32 API, this way we don't have to use pthread-win32 on Windows, which is a problem because it's LGPL-licenced.

@SirLynix SirLynix changed the title Fix compilation with MSVC14 and C++14 compilers Fix compilation with MSVC14 and C++14 compilers, and gets rids of pthread Oct 13, 2016
@SirLynix SirLynix changed the title Fix compilation with MSVC14 and C++14 compilers, and gets rids of pthread Fix compilation with MSVC14 and C++14 compilers, and gets rid of pthread-win32 Oct 13, 2016
@SirLynix
Copy link
Contributor Author

SirLynix commented Nov 4, 2016

Just realized I fixed that with C++ (which I didn't see because I compiled it first with MSVC), I pushed a commit which convert this to C code.

It's now compiling with MinGW as well!

@roig
Copy link

roig commented Mar 2, 2017

Thank you for this pull request I hope this will get merged soon..

@SirLynix
Copy link
Contributor Author

SirLynix commented Apr 2, 2017

Hello? Is this repository still maintained?

@andykorth
Copy link
Collaborator

The big issue with platform specific compilation issues is that the fixes created by one person tend to break the other platforms. They require a lot of time consuming testing, which often involves setting up a windows VM, downloading and installing developer tools, and figuring them out. Chipmunk will probably get some attention in the next few weeks once our current project is done.

@Alan-FGR
Copy link

Alan-FGR commented Aug 1, 2017

@DrLynix did you guys test you pthread implementation?

I'm happily using pthread-win32 (LGPL can be linked dynamically without affecting your sauce), but decided it was time to give this a try, however, it doesn't work for me... I'm on VS2015, it builds fine (64 bit), but when I try to run it, I get this error (it's a simple program similar to the 'hello world' example):

Exception thrown at 0x0000000077B7AC04 (ntdll.dll) in chipmunk.exe: 0xC0000005: Access violation writing location 0x0000000000000024.
E pthread_cond_broadcast(pthread_cond_t * cv=0x00000000003a4b00)                         Line 85    
  HaltThreads(cpHastySpace * hasty=0x00000000003a4890)                                   Line 496
  cpHastySpaceSetThreads(cpSpace * space=0x00000000003a4890, unsigned long threads=1)    Line 521
  cpHastySpaceNew()                                                                      Line 567
  main()                                                                                 Line 68  

Edit: It's the same code that works perfectly fine with pthread-win32.

@SirLynix
Copy link
Contributor Author

SirLynix commented Aug 1, 2017 via email

@Alan-FGR
Copy link

Alan-FGR commented Aug 1, 2017

Wow, thank you for the quick reply! No need to hurry though, I'm OK using pthread-win32, but I'd rather get rid of it if possible.

@SirLynix
Copy link
Contributor Author

SirLynix commented Aug 2, 2017

There, fixed 😅

@Alan-FGR
Copy link

Alan-FGR commented Aug 3, 2017

Thank you. It's working fine. Do you have any ideas if performance should be comparable to pthreads-win32 or should I benchmark it?

@SirLynix
Copy link
Contributor Author

SirLynix commented Aug 3, 2017

From what I saw in pthread-w32 source code, it differs by its implementation (by using atomic locks instead of critical sections) so I don't really know.

It would be really nice if you could benchmark it to be sure (I have a few ideas on how to optimize this implementation just in case).

@slembcke
Copy link
Owner

This is way overdue. Sorry about that.

To add to the discussion about threading: Chipmunk was originally only designed so that it would be safe to run separate spaces in separate threads (no globals). Threading the solver with fine grained locking (ex: per body) wouldn't really work, so it really just embraces race conditions. If one thread overwrites the work of another, that's sort ok since that's sort of how the iterative solver works anyway. Also, because the access patterns are fairly random, race conditions are fairly rare.

If that sounds like a massive hack, it's because it is... While I've thrown a CPU week or so at testing for issues, I really have no idea if it's possible for it to have rare, but catastrophic issues. The only platform where it seems to have issues was the 32 bit iOS simulator, where it failed almost instantly.

Long story longer... The other problem with the way it's threaded is that it seems to scale really poorly past 2 threads. Not really sure why, maybe due to the random memory access patterns or something. To get a really good, scalable, threadable, SIMDable solver, it's going to require a rewrite of a significant portion of Chipmunk. Realistically that's not going to happen without creating a brand new library. :-\

@slembcke slembcke merged commit d13c0d9 into slembcke:master Aug 25, 2017
@Alan-FGR
Copy link

Alan-FGR commented Aug 25, 2017

@slembcke from my experience, it doesn't even scale well on 2 threads in many cases, and in fact I've seen a lot of scenarios where it was substantially faster single-threaded; CPU core usage for some reason is always subpar too, but I think in most typical cases the MT solver is more performant, but marginally so.
I'd really like to benchmark the solution here compared to pthreads-win32, but I had a disk failure so that's going to take a while; hopefully the implementation by @DrLynix is faster :) but you never know.
Other than that, I'm really pleased with Chipmunk so far. While Box2D is in my opinion a 'reference' implementation, which is good to learn some tricks from, this is the real deal for production imho, it's the de facto industrial 2D physics engine, and it's awesomely done, and easily bindable from other languages to experience it in many flavors! Awesome work, seriously awesome!

@slembcke
Copy link
Owner

Maybe a good way to sum up what I was trying to say is that the threaded solver works, but I'm trying to encourage people not to become dependent on it.

Other than that, I'm really pleased with Chipmunk

Thanks! I've put a lot of hours into it in the last... almost 11 years! (Now I feel old. Haha) It's always good to see that people still find it useful.

@Alan-FGR
Copy link

Maybe a good way to sum up what I was trying to say is that the threaded solver works, but I'm trying to encourage people not to become dependent on it.

Yes, in some specific situations it scales outstandingly well though, especially when you have large simulations with many iterations. You're right though, and I would encourage people to just try both and see what works best for their use case.

Thanks! I've put a lot of hours into it in the last... almost 11 years! (Now I feel old. Haha) It's always good to see that people still find it useful.

Well, they say you're only as old as you feel :trollface:... but really, Chipmunk is amazing and I still recommend it to everybody! It's certainly something to be extremely proud of!

@SirLynix
Copy link
Contributor Author

@slembcke Nice to see you back.
Since we use chipmunk at work, I can use some of my time to help you with other pull requests but I don't have a lot of physics/math skills, but I can help you with the code!

@vonj
Copy link

vonj commented Aug 25, 2017 via email

@Alan-FGR
Copy link

Alan-FGR commented Aug 25, 2017

@vonj Yes, this doesn't affect the single-threaded solver at all. For most cases you can safely use the normal cpSpace (not cpHastySpace) and still have performance superior to pretty much anything else.
EDIT: As mentioned below, yep, the hasty space has hardware acceleration on mobile platforms so you may want to use it there in single-threaded mode, but nothing in this PR affects that.

@slembcke
Copy link
Owner

slembcke commented Aug 25, 2017

@vonj Do you mean the 32 bit iPhone sim comment? The single threaded solver doesn't use any weird hacks like that. I'm pretty sure the way my threaded one works is violating some of C's memory model (accessing shared memory without a lock/barrier/atomics) that is relaxed on many actual platforms.

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

6 participants