Repository navigation
Make Project.WalkTodosOfDir concurrent - #166
Conversation
Use built-in packages like `sync` and `context` instead. Remove FIXME for "too many open files" error. I replaced the workers count (1024) to instead use the available logical CPUs with `runtime.NumCpu()` for a more reasonable size. With this approach that error will never occur. Benchmarking the list command with both approaches in the open source `minetest` project using the `time` command, It went from 7.8s to 4.3s on my machine (Intel i5-8400).
rexim
left a comment
rexim
left a comment
There was a problem hiding this comment.
Can we just make Project.WalkTodosOfDir signature look like this?
func (project Project) WalkTodosOfDir(dirpath string) chan (Todo, error)The subcommands keep pulling todos from the channel and if they encounter an error they simply close the channel which will indicate the cancellation for the jobs that push into the channel.
Whatever we are doing here right now feels like we are trying to fit a cube into a circle hole. :D
|
@aod I really apologize for being slow on this pull request, just busy with other things. Just wanted to say that I didn't forget about it and will get back to it as soon as I get a chance. |
|
@aod hey! Sorry that it took so long for me to find time to sit down and properly take a look at your Pull Request. I wish the end result was a bit less complex, but I could not find a simpler solution myself. So it looks good to me for now! 👍 Thank you so much for you contribution and patience! |
2 participants
Possibly fixes #153
Only
snitch purgeandsnitch reportare partly concurrent, the HTTP requests remain sequential. This could be made concurrent as well but maybe not desired.I've implemented this with a worker pool like pattern. Limiting the amount of files being walked at a given time. Since it will report an error about exceeding the open file descriptors limit on very large projects. I've hardcoded this value for now to the default soft limit on linux. But maybe we should get the value from
ulimitand use that instead? (What about other platforms?)Furthermore I noticed that snitch doesn't ignore non-source-code paths e.g. image paths. I've put a FIXME above the line. Possibly have an option in the config for blacklisting file extensions.