libgit2 / docurium

Doxygen replacement for the libgit2 project
http://libgit2.github.com/libgit2
MIT License
158 stars 25 forks source link

Switch to parallel gem #43

Closed tiennou closed 4 years ago

tiennou commented 5 years ago

26 originally removed all traces of parallelism in favor of less file-descriptor contention. It turns out Ruby has gems, and some of those provide ready-made, easy-to-use wrappers for parallel processing.

Use https://github.com/grosser/parallel instead of rolling our own, or removing parallelization altogether.

(Note that this is based on #39, for historic reasons. I can rebase that on top of master if it makes it easier to go in).

carlosmn commented 5 years ago

Yeah I'd rather see this against master, otherwise there's random changes that make it hard to see what's actually changing here.

Is file descriptor contention an issue you're seeing with the current multi-process scheme rather than oversubscription of the processors?

tiennou commented 5 years ago

I'm pretty sure the issue is that the current master code does is :

I remember getting a sluggish machine, and it would sometimes start throwing "Too many open file descriptors errors" out of this. Don't know if it's CPU contention, SSD thrashing or whatever, but docurium was definitely mostly-unusable here, as there are no guarantee any run will be a complete success, thus sometimes causing complete versions to be missing from the final repository.

carlosmn commented 5 years ago

Right, I was curious as to how you pinpointed file description contention as the source of the issue. I'm pretty sure it's due to CPU oversaturation, but I'd be impressed if we managed to thrash on an SSD ;)

The issue with too many open files is most likely due to lack of garbage collection on the repository, unless your ulimits are low.

This gem seems really useful, but I'd still want the ability to set how many processes I want to run.

tiennou commented 5 years ago

It's possible, via either in_processes: or in_threads:. But I'd like to post-pone that to at least the option support (part of #44), and the auto-detection makes it less intrusive.

carlosmn commented 4 years ago

It looks like we had some mismerges into master but as I'm currently restricted to my work laptop, the master branch makes it really unhappy so a patch is gonna make it here to fix things