3

I have a Semaphore that suspose to limit to 3, however, this just keeps calling as many as it wants. I'm assuming it's because I use (1000). However, when I try just () it will never pass the WaitOne i'm not sure what to do here.

private static Semaphore _pool; _pool = new Semaphore(0, 3); var options = new ParallelOptions(); options.MaxDegreeOfParallelism = 1; Parallel.ForEach(urlTable.AsEnumerable(),options, drow => { using (var WCC = new MasterCrawlerClass()) { ActiveThreads++; _pool.WaitOne(1000); Console.WriteLine("Active Thread #: " + ActiveThreads); WCC.MasterCrawlBegin(drow); Console.WriteLine("Done Crawling a datarow"); ActiveThreads--; _pool.Release(); } });
Improve this question
4
  • Seeing as you're setting maxdop to 1, what does the use of Parallel.ForEach actually buy you here? The loop body will also probably be running in the ThreadPool, where blocking code is a big no-no. You should also be guarding against concurrent access to ActiveThreads. Can you explain what you're trying to do here? There's probably a better way. Commented Apr 12, 2012 at 0:38
  • @spender All i want to be able to do is limit the number of times the Parallel.Foreach actually calls WCC Commented Apr 12, 2012 at 0:45
  • Limit to what? At most 3 concurrent crawls? Commented Apr 12, 2012 at 0:49
  • @spender yes that would be fine, but i really just want to say int = ; and whatever it is limit to that Commented Apr 12, 2012 at 1:21

2 Answers 2

Reset to default
7

You are miscontructing the semaphore. It should be _pool = new Semaphore(3, 3); Doing that will also eliminate the need to pass in a timeout parameter to WaitOne().

The first parameter is the initial number of requests that may be granted before blocking, so passing 0 means any subsequent calls to WaitOne() will immediately block.

Improve this answer
Sign up to request clarification or add additional context in comments.

4 Comments

It might just be because I dont understand, but isnt the first Int intial count? I want to have 0, and then continue until I have 3, and the second int is max, so should it be (0,3)
@Mike It's the initial count that may be granted; setting it to 0 means "you can initially grant 0 requests before blocking", which is obviously not what you want.
This might not be what I want, in my foreach loop it only goes into one loop regardless if I change it to (3,3) or (4,8) why is that
@Mike: Because you set MaxDegreeOfParallelism = 1?
3

There are a few problems here.

  • I suspect there is little reason to throttle concurrency in this situation in the first place.
  • You are setting initialCount to 0 in the Semaphore constructor. initialCount is the number of requests that can be granted right now.
  • You are using the overload of WaitOne that accepts a timeout parameter. When the timeout expires WaitOne will return regardless of whether a count was taken from the semaphore.
  • You are incrementing ActiveThreads before taking a count from the semaphore. This means that ActiveThreads more closely approximates the number of simultaneous worker threads for the ForEach operation. It will not tell you how many of those threads are executing MasterCrawlBegin.
  • The ++ operation on ActiveThreads is not thread-safe.
  • You set MaxDegreesOfParallelism to a value that pretty much eliminates any kind of concurrent processing.

Change your code so that it looks like this.

int ActiveThreads = 0; int ActiveCrawls = 0; var semaphore = new SemaphoreSlim(3, 3); Parallel.ForEach(urlTable.AsEnumerable(), drow => { int x = Interlocked.Increment(ref ActiveThreads); Console.WriteLine("Active Thread #: " + x); semaphore.Wait(); int y = Interlocked.Increment(ref ActiveCrawls); Console.WriteLine("Active Crawl #: " + y); try { using (var WCC = new MasterCrawlerClass()) { WCC.MasterCrawlBegin(drow); } } finally { Interlocked.Decrement(ref ActiveCrawls); semaphore.Release(); Interlocked.Decrement(ref ActiveThreads); Console.WriteLine("Done Crawling a datarow"); } } });

And of course you can just set MaxDegreesOfParallelism = 3 and not worry about all the semaphore stuff.

Improve this answer

1 Comment

Refer to my answer here for further analysis of your problem and a reason why I think throttling parallelism is a bad idea in this situation.