[PATCH] D148728: [Support][Parallel] Add sequential mode to TaskGroup::spawn().

[Alexey Lapshin via Phabricator via llvm-commits]

avl added inline comments.


================
Comment at: llvm/lib/Support/Parallel.cpp:16
 #include <future>
-#include <stack>
+#include <queue>
 #include <thread>
----------------
avl wrote:
> andrewng wrote:
> > Unfortunately, at least in my testing on Windows, this change causes a drop in performance of around 3-4%. I believe that's why `std::stack` is used.
> > Unfortunately, at least in my testing on Windows, this change causes a drop in performance of around 3-4%. I believe that's why `std::stack` is used.
> 
> will check it.
>Unfortunately, at least in my testing on Windows, this change causes a drop in performance of around 3-4%. I believe that's why std::stack is used.

yes, there is small performance degradation in case std::queue is used instead of std::stack.
But, to have it noticeable, there should be created very large amount of tasks.
For the 100000000 tasks the std::queue works 8sec slower:

```

llvm::parallel::TaskGroup tg;
for (size_t Idx = 0; Idx < 100000000; Idx++) {
  tg.spawn([](){});
}

                        Time(sec)  Memory(kB)

queue TaskGroup          121.28    441568

stack TaskGroup          113.61    471820
```

Generally, it is a bad idea to create such amount of tasks.
parallelFor has optimization to not create a lot of tasks.
There is no any performance difference if parallelFor is used:

```

parallelFor(0,100000000,[](size_t){});

                        Time(sec)  Memory(kB)

queue parallelFor          0.03    9128

stack parallelFor          0.03    9128
```

So, probably using std::queue is OK.

Or, Dou you have a test case when using queue still bad idea?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D148728/new/

https://reviews.llvm.org/D148728