c4milo / fusejs

Low level fuse bindings for nodejs
http://c4milo.github.io/fusejs
111 stars 72 forks source link

Eventually remove ck dependency in favor of our own circular buffer implementation. #19

Closed c4milo closed 9 years ago

c4milo commented 9 years ago

It could also be extracting that specific part of CK and statically compile it with the rest of the addon, as opposed to having the whole library as a dependency. Any thoughts @thejinx0r?

EricTheMagician commented 9 years ago

I'm all for it. It would make distribution / compilation easier by removing CK as a dependency.

I've never had to ensure thread safety with queues, so I sought out for solutions because I didn't want to screw it up. If you have any suggestions, on which implementation to use, let me know. I'm starting to notice cracks with CK and I'd rather change soon rather later, especially since I'm using the wrong type's of queues.

I think I might go with this one: https://github.com/mstump/queues/blob/master/include/mpsc-queue.hpp

c4milo commented 9 years ago

it doesn't seem to be circular.

c4milo commented 9 years ago

I would say, let's go with a regular implementation, if we ever hit an issue due to lock contention, we then can look into the lock-free circular queue in more detail as it doesn't seem to be that simple.

EricTheMagician commented 9 years ago

https://github.com/fsaintjacques/disruptor--/blob/develop/disruptor/ring_buffer.h Probably.

c4milo commented 9 years ago

@thejinx0r yes! Disruptor FTW.

EricTheMagician commented 9 years ago

I started exploring https://github.com/redjack/varon-t/ in favor of disruptor--, since it was more feature complete.

One oversight with disruptor is that each producer/consumer needs to be contained within a thread. This does not play well with design principles of nodejs/iojs. The event loop of nodejs is run in one thread, which the consumer must be in. But this consumer will block the thread and prevent setTimeouts from running.

In the meantime, I have implemented a simple MPSC queue and enabled multithreaded by default.

c4milo commented 9 years ago

That's a reasonable first step. Nice work @thejinx0r!