0

In a long running Windows Service I have a custom thread pool manager that has a loop which continuously looks through a couple lists for idle threads.

Given multiple uses of the same predicate, is there any benefit to saving off the lambda as a Func?
How might the recommendation change based on the count of threads this code is interacting with, or when the Func might need to be passed to other methods to help with the work?

while (_isRunning )
{
  ...sleep a bit to prevent thrashing
  while ( _threads.Any(t => t.IsIdle) || _autreThreads.Any(t => t.IsIdle)
  {
    var assignee = _threads.FirstOrDefault(t => t.IsIdle) ?? _autreThreads.FirstOrDefault(t => t.IsIdle);
    ...give 'em something to do 
  }
}

In place of each of the lambdas I could use the following, defined once outside for the loop, for reuse.

Func<WorkerThread, bool> isIdleCondition = t => t.IsIdle;

3 Answers 3

2

Note that t => t.IsIdle is as much a literal constant as 5 or "Hello world" is. It never changes, the compiler sees that, and thus knows that the func is a constant that doesn't need to be re-instantiated every time the method is executed.


How might the recommendation change based on the count of threads this code is interacting with, or when the Func might need to be passed to other methods to help with the work?

It doesn't really. Even if put into a separate property, you're only ever going to get this property, never set it, so there won't be an issue as the func will always be available and race conditions are non-existant.


In place of each of the lambdas I could use the following, defined once outside for the loop, for reuse.

The only reason to put this in a property if for the sake of readability and reusability. There is no meaningful difference for runtime performance.

So I would still advocate putting it into a property of its own, but not for the reasons you're asking about in your question.

3

I think you are focusing your attention in the wrong place to start with.

while ( _threads.Any(t => t.IsIdle) || _autreThreads.Any(t => t.IsIdle) )
{
    var assignee = _threads.FirstOrDefault(t => t.IsIdle) ?? _autreThreads.FirstOrDefault(t => t.IsIdle);
    ...give 'em something to do 
}

Is essentially doing double work, first to check the existance of an idle thread. _threads.Any(t => t.IsIdle) || _autreThreads.Any(t => t.IsIdle) then to get that thread _threads.FirstOrDefault(t => t.IsIdle) ?? _autreThreads.FirstOrDefault(t => t.IsIdle);

You can cut the work in half by doing this:

ThreadType assignee = null
while (null != (assignee = _threads.FirstOrDefault(t => t.IsIdle) ?? _autreThreads.FirstOrDefault(t => t.IsIdle)))
{
    ...give 'em something to do 
}

Now we find and grab the idle thread in one step.

Still there are other issues here, so lets avoid attacking the lambda, and instead ask why you have to iterate those two lists independently?

ThreadType assignee = null
while (null != (assignee = _threads.Union(_autreThreads).FirstOrDefault(t => t.IsIdle)))
{
    ...give 'em something to do 
}

Now we lazy evaluate both lists, finding the first idle thread.

You could push the lambda into a function now, but why do the compilers job?

3
  • 2
    or just for (var assignee in _threads.Union(_autreThreads).Where(t => t.IsIdle))
    – Caleth
    Nov 29, 2018 at 9:41
  • Union or Concat options were another question i had regarding performance. Since i could not find any concrete answers regarding their performance, and the conditions can be short-circuited without needing to evaluate the second list.
    – rediVider
    Nov 29, 2018 at 14:20
  • I tried an inline variable definition in the conditional, like the newish inline out parameter, but it looks like its now supported [yet]. Somehow i didn't make the leap to initializing the variable before the loop.
    – rediVider
    Nov 29, 2018 at 14:31
0

It can help slightly with readability in some cases, and it can help occasionally if there is a change so you don’t have to fix two places.

But it can also occasionally lead to problems when you change one place and impact another unexpectedly.

For this example, it seems like overkill.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.