Re: [PATCH 1/4] Avoid submitting empty tasks in parallel_for_each

[Joel Brobecker <brobecker@adacore.com>]

Hi Tom,

On Thu, Dec 15, 2022 at 12:07:56PM -0700, Tom Tromey via Gdb-patches wrote:
> I found that parallel_for_each would submit empty tasks to the thread
> pool.  For example, this can happen if the number of tasks is smaller
> than the number of available threads.  In the DWARF reader, this
> resulted in the cooked index containing empty sub-indices.  This patch
> arranges to instead shrink the result vector and process the trailing
> entries in the calling thread.

This patch looks good to me.

If I'm allowed to nitpick...

> ---
>  gdbsupport/parallel-for.h | 23 +++++++++++++++++++++++
>  1 file changed, 23 insertions(+)
> 
> diff --git a/gdbsupport/parallel-for.h b/gdbsupport/parallel-for.h
> index acd9137efbd..ed460f31f63 100644
> --- a/gdbsupport/parallel-for.h
> +++ b/gdbsupport/parallel-for.h
> @@ -70,6 +70,12 @@ struct par_for_accumulator
>      return result;
>    }
>  
> +  /* Resize the results to N.  */
> +  void resize (size_t n)
> +  {
> +    m_futures.resize (n);
> +  }
> +
>  private:
>    
>    /* A vector of futures coming from the tasks run in the
> @@ -108,6 +114,12 @@ struct par_for_accumulator<void>
>        }
>    }
>  
> +  /* Resize the results to N.  */
> +  void resize (size_t n)
> +  {
> +    m_futures.resize (n);
> +  }
> +
>  private:
>  
>    std::vector<gdb::future<void>> m_futures;
> @@ -232,6 +244,17 @@ parallel_for_each (unsigned n, RandomIt first, RandomIt last,
>  	  end = j;
>  	  remaining_size -= chunk_size;
>  	}
> +
> +      if (end == last)
> +	{
> +	  /* We're about to dispatch the last batch of elements, which
> +	     we normally process in the main thread.  So just truncate
> +	     the result list here.  */

... I'd expand the last sentence to explain that this is to avoid
submitting empty tasks to the thread pool.

But I'll leave it up to you do decide whether that's a useful suggestion
or not ;-).

Thanks for that patch!

> +	  count = i;
> +	  results.resize (count);
> +	  break;
> +	}
> +
>        if (parallel_for_each_debug)
>  	{
>  	  debug_printf (_("Parallel for: elements on worker thread %i\t: %zu"),
> -- 
> 2.34.3
> 

-- 
Joel