-
-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Banish all locks #1053
Banish all locks #1053
Conversation
There seems to be something, but I'm not sure if it's caused by this commit specifically or something earlier... happens with live notes mostly, causes segfaults with broken stack. Someone want to figure out why this happens? |
Some Windows 32-bit: Windows 64-bit: Vesa, I may not have time to test this for a little while. Feel free to bump it in a few days if you receive no testing. |
Ok, I think I'm going to merge this, because it fixes many instability issues with the current master, and even if there are some new issues in exchange (probably), it's still a good trade as we have to keep moving forward. |
Looks good! |
oh btw @tobydox (Incidentally, we could probably also remove all the intel compiler stuff from the codebase, to clean things up a bit) |
Yeah I noticed and I think this should be fine. If not, we could build a wrapper around it which allocates the data on stack or heap depending on compiler. When using the stack we should still keep in mind that the available stack memory can be very limited on some platforms so it's not a good idea to allocate several megabytes of memory on the stack (small buffers with a few KB is no problem I think). Otherwise this can cause bugs like #543 which has been fixed via 9c27956. |
On 08/16/2014 03:04 PM, Tobias Doerffel wrote:
Yep, the problem is that heap allocations are apparently bad because In the long run I'm thinking we can solve this issue by creating a |
Yeah I know, heap allocation is bad for RT. A dedicated lightweight memory manager is what we had long time ago IIRC but should be worth to be implemented again. |
On 08/16/2014 03:21 PM, Tobias Doerffel wrote:
Sounds interesting... Can the source code of this memory manager still |
It existed up to version 0.2.1: https://github.com/LMMS/lmms/blob/d88b2959ceb0a0f714f5bea60b53ba255f2dc661/include/buffer_allocator.h No idea whether we can still re-use it. Probably it's better to write something from scratch. |
On 08/20/2014 07:58 PM, Tobias Doerffel wrote:
Hmm, yeah, I took a quick look at it and I'm not sure if it's exactly |
After a bit more looking into it, it looks like the old LMMS memory I think what we need is rather something which at first allocates a I found this quite nice tutorial for writing customized memory managers: Maybe the approach described there could be useful for our purposes? |
Yes this is probably a better idea :) |
More removal of global lock calls: mostly replacing them with local mutexes (eh.. mutices?) which don't block the core rendering function.
A smart (I hope) thing I did was adding "processing locks" to tracks and playhandles. This way, when tracks are busy doing something like loading a plugin, they don't have to block the mixer thread: they just lock their "processing lock", which causes the play function to skip processing in tracks. Playhandles don't skip, they block but only their own rendering thread.
I think I can even notice some performance boost, although I'm not sure about that, haven't benchmarked or anything, but some projects of mine that are very cpu-heavy seem to run a bit smoother in this branch, compared to current master.
@tresf, @softrabbit, @grejppi, some testing of this branch would be nice before merging it.