[Lldb-commits] [PATCH] D32757: Add TaskMap for iterating a function over a set of integers
Scott Smith via Phabricator via lldb-commits
lldb-commits at lists.llvm.org
Tue May 2 13:48:18 PDT 2017
scott.smith marked 6 inline comments as done.
scott.smith added a comment.
IMO the vector append issue doesn't matter, because the very next thing we do is sort. Sorting is more expensive than appending, so the append is noise.
================
Comment at: source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:1994
- TaskRunner<uint32_t> task_runner;
- for (uint32_t cu_idx = 0; cu_idx < num_compile_units; ++cu_idx)
- task_runner.AddTask(parser_fn, cu_idx);
-
- while (true) {
- std::future<uint32_t> f = task_runner.WaitForNextCompletedTask();
- if (!f.valid())
- break;
- uint32_t cu_idx = f.get();
+ TaskMap(num_compile_units, 1, parser_fn);
----------------
scott.smith wrote:
> clayborg wrote:
> > Note that we lost performance here. The old code would run:
> >
> > ```
> > while (true) {
> > std::future<uint32_t> f = task_runner.WaitForNextCompletedTask();
> > // Do expensive work as soon as possible with any random task that completes as soon as it completes.
> > ```
> >
> > Your current code says "wait to do all expensive work until I complete everything and then do all of the expensive work".
> >
> >
> >
> >
> The following loop is not expensive, it's simple vector concatenation of fairly simple types. The actual slow work is Finalize(), which calls Sort().
>
> m_function_basename_index is of type NameDIE, which has a UniqueCStringMap member, which declared collection as std::vector.
>
> Though arguably the Append should be pushed down into the RunTasks below.
>
This takes <0.25s (out of a total of 15 seconds of runtime) when timing lldb starting lldb (RelWithDebInfo build). That's for an aggregate of 14M+ names being added to the vectors. IMO that should not block this change.
I also moved the append into RunTasks, just because we already have those subtasks.
================
Comment at: source/Utility/TaskPool.cpp:77
+
+void TaskMap(size_t size, size_t batch, std::function<void(size_t)> const & func)
+{
----------------
clayborg wrote:
> Is this named correctly? Maybe SerializedTaskMap? Or BatchedTaskMap?
TaskMapOverInt?
Repository:
rL LLVM
https://reviews.llvm.org/D32757
More information about the lldb-commits
mailing list