Add concurrency #12
Add concurrency #12
Conversation
Signed-off-by: Denys Smirnov <denys@sourced.tech>
Signed-off-by: Denys Smirnov <denys@sourced.tech>
extractor.go
Outdated
| jobs = make(chan *extractJob, e.num) | ||
| errc = make(chan error, e.num) | ||
| for i := 0; i < e.num; i++ { | ||
| wg.Add(1) | ||
| go func() { | ||
| defer wg.Done() | ||
| e.worker(jobs, errc) | ||
| }() | ||
| } | ||
| // no sample buffer, it's per-routine |
There was a problem hiding this comment.
Lets move it to some function.
Btw. I think error channel is not closed.
There was a problem hiding this comment.
Sorry, but I don't think it will make the code smaller.
If you propose to move the worker creation, it will require to pass wg to a function, which is considered a bad practice (you should see all usages of wg in one place, if possible).
Same for other things: creating a function will require passing other state like channels to them. Having in mind the size of the code inlining it here may still be acceptable.
But if you have some specific suggestion, please elaborate a bit more.
Re: not closing the channel - it's not necessary. GC will take care of that anyway. It's not a resource leak.
There was a problem hiding this comment.
my intention was not make the code shorter but more readable (less statements from different contexts, like here we start workers, there we handle channels plus ifs single threaded or multi threaded.
There was a problem hiding this comment.
As I said, if you have any suggestions - please elaborate. Moving things will complicate code or will require passing all the state as arguments.
|
here are some tests with ~3K C# files: concurrent: OK, it's better if you have (4CPUs), but otherwise it's not a huge performance improvement. I wonder if it's a json IO + mutex or maybe communication over channels. |
|
@kuba-- I don't think you are testing the same thing, though. |
|
@dennwc it's not really because of sampling. |
Signed-off-by: Denys Smirnov <denys@sourced.tech>
Signed-off-by: Denys Smirnov <denys@sourced.tech>
This change actually uses the
-nparameter to increase concurrency for CLI. Also, reduce the sample size and add a temporary workaround for the parser lifetime bug in the bindings.This change is