async_simple icon indicating copy to clipboard operation
async_simple copied to clipboard

optimize lazy collectAll implement

Open RainMark opened this issue 3 years ago • 5 comments

Search before asking

  • [ ] I searched the issues and found no similar issues.

What happened + What you expected to happen

https://github.com/alibaba/async_simple/blob/ab70389831cb2b9be060fd66c913833c000843fc/async_simple/coro/Collect.h#L229

Lazy<> foo() {}

collectAll(foo().via(executor)) is equal to collectAllPara(foo())

maybe we can use collectAll(foo().via(executor)) to simplify the collectAllPara, for example: collectAllPara call collectAll(foo().via(executor)) directly

Reproduction way

Anything else

Are you willing to submit a PR?

  • [ ] Yes I am willing to submit a PR!

RainMark avatar Jul 06 '22 09:07 RainMark

What if the lazy don't contain an Executor when we call collectAllPara(foo())?

ChuanqiXu9 avatar Jul 07 '22 02:07 ChuanqiXu9

What if the lazy don't contain an Executor when we call collectAllPara(foo())?

collectAllPara can not run sub coroutines in parallel without executor

RainMark avatar Jul 07 '22 03:07 RainMark

Another problem is that collectAllPara is not equal to collectAll, it might be design mistake. We may need to change it.

ChuanqiXu9 avatar Jul 07 '22 03:07 ChuanqiXu9

Another problem is that collectAllPara is not equal to collectAll, it might be design mistake. We may need to change it.

em,we can remove

using AT = std::conditional_t<
        std::is_same_v<LazyType<T>, Lazy<T>>,
        detail::SimpleCollectAllAwaitable<T, IAlloc, OAlloc, Para>,
        detail::CollectAllAwaiter<LazyType<T>, IAlloc, OAlloc, Para>>;

RainMark avatar Jul 07 '22 04:07 RainMark

Another problem is that collectAllPara is not equal to collectAll, it might be design mistake. We may need to change it.

em,we can remove

using AT = std::conditional_t<
        std::is_same_v<LazyType<T>, Lazy<T>>,
        detail::SimpleCollectAllAwaitable<T, IAlloc, OAlloc, Para>,
        detail::CollectAllAwaiter<LazyType<T>, IAlloc, OAlloc, Para>>;

Maybe we need to do more work on the interface. The previous design looks not good.

ChuanqiXu9 avatar Jul 07 '22 05:07 ChuanqiXu9