Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor launch #173

Merged
merged 1 commit into from
Nov 3, 2022
Merged

Refactor launch #173

merged 1 commit into from
Nov 3, 2022

Conversation

4kangjc
Copy link
Collaborator

@4kangjc 4kangjc commented Oct 29, 2022

Why

#171

What is changing

重构uthread::Launch,修改为enum class

Example

#171

@CLAassistant
Copy link

CLAassistant commented Oct 29, 2022

CLA assistant check
All committers have signed the CLA.

@yanminhui
Copy link
Collaborator

启动类型不必作为模板参数,这么做使得各个 async 重载变得复杂,需要长的模板参数判断(std::enable_if),调用者还要指定模板参数,可以参照标准库 std::async 类似做法,让它作为函数参数,实现看起来更简洁,调用者也不必指定模板参数。

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

启动类型不必作为模板参数,这么做使得各个 async 重载变得复杂,需要长的模板参数判断(std::enable_if),调用者还要指定模板参数,可以参照标准库 std::async 类似做法,让它作为函数参数,实现看起来更简洁,调用者也不必指定模板参数。

主要考虑到接口不变的情况下,如果接口改变的话,可以试试

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

启动类型不必作为模板参数,这么做使得各个 async 重载变得复杂,需要长的模板参数判断(std::enable_if),调用者还要指定模板参数,可以参照标准库 std::async 类似做法,让它作为函数参数,实现看起来更简洁,调用者也不必指定模板参数。

还有就是如果作为函数参数是运行期的,作为模板参数是编译期的

@yanminhui
Copy link
Collaborator

还有就是如果作为函数参数是运行期的,作为模板参数是编译期的

忽略不计,否则许多传递枚举类型的函数都得改成作为模板参数。

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

还有就是如果作为函数参数是运行期的,作为模板参数是编译期的

忽略不计,否则许多传递枚举类型的函数都得改成作为模板参数。

可是返回值得编译期确定,当LaunchPrompt类型时,返回值为Uthread,如何与其他类型区分呢?

@yanminhui
Copy link
Collaborator

可是返回值得编译期确定,当LaunchPrompt类型时,返回值为Uthread,如何与其他类型区分呢?

一般来说,重载的函数应具有相同的返回值类型,如果发现返回值类型可能不同,返回值可以作为出参,也就是说把返回值移到出参上,或者重新命名一个新的函数名进行区分。

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

可是返回值得编译期确定,当LaunchPrompt类型时,返回值为Uthread,如何与其他类型区分呢?

一般来说,重载的函数应具有相同的返回值类型,如果发现返回值类型可能不同,返回值可以作为出参,也就是说把返回值移到出参上,或者重新命名一个新的函数名进行区分。

这里并不适合,标准库那个可以做或运算,这里是互斥的,就算作为函数参数传递,调用者也得指定参数,并不比作为模版参数传递简洁,失去了最初的意义

@yanminhui
Copy link
Collaborator

这里并不适合,标准库那个可以做或运算,这里是互斥的,就算作为函数参数传递,调用者也得指定参数,并不比作为模版参数传递简洁,失去了最初的意义

按你这么说,std::async 也是互斥的,完全可以如下实现,也会是编译期。

// launch::async
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async, int> = 0>
auto async(_Fp&& __f, _Args&&... __args) {
    // ...
}

// launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::deferred, int> = 0>
auto async(_Fp&& __f, _Args&&... __args) {
    // ...
}

// launch::async | launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async|launch::deferred, int> = 0>
auto async(_Fp&& __f, _Args&&... __args) {
    return async<launch::async>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
    // or return async<launch::deferred>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
}

我们写模板函数为的是简洁,尽量能根据实参自动推导,不把显示实例化交给用户。

// compile-time
std::async<launch::deferred>(f, arg1, arg2);

// run-time
std::async(launch::deferred, f, arg1, arg2);

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

这里并不适合,标准库那个可以做或运算,这里是互斥的,就算作为函数参数传递,调用者也得指定参数,并不比作为模版参数传递简洁,失去了最初的意义

按你这么说,std::async 也是互斥的,完全可以如下实现,也会是编译期。

// launch::async
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async, int> = 0>
auto async(_Fp&& __f, _Args&&... __args) {
    // ...
}

// launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::deferred, int> = 0>
auto async(_Fp&& __f, _Args&&... __args) {
    // ...
}

// launch::async | launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async|launch::deferred, int> = 0>
auto async(_Fp&& __f, _Args&&... __args) {
    return async<launch::async>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
    // or return async<launch::deferred>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
}

我们写模板函数为的是简洁,尽量能根据实参自动推导,不把显示实例化交给用户。

// compile-time
std::async<launch::deferred>(f, arg1, arg2);

// run-time
std::async(launch::deferred, f, arg1, arg2);

launch::asyncaunch::deferred并不互斥啊,他们可以同时存在,Launch::PromptLaunch::ScheduleLaunch::Current不行

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

这里并不适合,标准库那个可以做或运算,这里是互斥的,就算作为函数参数传递,调用者也得指定参数,并不比作为模版参数传递简洁,失去了最初的意义

按你这么说,std::async 也是互斥的,完全可以如下实现,也会是编译期。

// launch::async
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async, int> = 0>
auto async(_Fp&& __f, _Args&&... __args) {
    // ...
}

// launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::deferred, int> = 0>
auto async(_Fp&& __f, _Args&&... __args) {
    // ...
}

// launch::async | launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async|launch::deferred, int> = 0>
auto async(_Fp&& __f, _Args&&... __args) {
    return async<launch::async>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
    // or return async<launch::deferred>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
}

我们写模板函数为的是简洁,尽量能根据实参自动推导,不把显示实例化交给用户。

// compile-time
std::async<launch::deferred>(f, arg1, arg2);

// run-time
std::async(launch::deferred, f, arg1, arg2);

launch::asyncaunch::deferred并不互斥啊,他们可以同时存在,Launch::PromptLaunch::ScheduleLaunch::Current不行

std::async<launch::async|launch::deferred>(f, arg1, arg2); 不等同于

async<launch::async>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...); 
async<launch::deferred>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);

所以如果通过模板参数的话,将来拓展也好,还是编写std::async<launch::async|launch::deferred>会有大量的冗余代码,所以标准库这个不适合使用模板参数传递async的类型,而async_simple是互斥的,并且返回值,函数的参数也不一样,例如Lauch::Schedule有两个版本,有一个和另外三个参数不一样,多一个C&& c,并不适合改写成函数参数传递

@yanminhui
Copy link
Collaborator

launch::asyncaunch::deferred并不互斥啊,他们可以同时存在,Launch::PromptLaunch::ScheduleLaunch::Current不行

互斥的,当你指定 async|deferred 时,意思是由系统选择其中的某个,最终还是走向 asyncdeferred

如果你觉得不互斥,可以看看上面的编译期声明会不会有问题。

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

launch::asyncaunch::deferred并不互斥啊,他们可以同时存在,Launch::PromptLaunch::ScheduleLaunch::Current不行

互斥的,当你指定 async|deferred 时,意思是由系统选择其中的某个,最终还是走向 asyncdeferred

如果你觉得不互斥,可以看看上面的编译期声明会不会有问题。

噢这样,没具体用过async,感觉怪怪的

@yanminhui
Copy link
Collaborator

你这里的实现编译通不过的,可以试着编译一下,得把模板写到返回值上

伪代码,若要编译如下:

#include <future>
#include <type_traits>

using namespace std;

// launch::async
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::deferred, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::async | launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == static_cast<launch>(launch::async|launch::deferred), int> = 0>
auto async2(_Fp&& __f, _Args&&... __args) {
    return async2<launch::async>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
    // or return async<launch::deferred>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
}

int main() {
    async2<launch::async>([](){});
    async2<launch::deferred>([](){});
    async2<launch::async|launch::deferred>([](){});
}

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

launch::asyncaunch::deferred并不互斥啊,他们可以同时存在,Launch::PromptLaunch::ScheduleLaunch::Current不行

互斥的,当你指定 async|deferred 时,意思是由系统选择其中的某个,最终还是走向 asyncdeferred
如果你觉得不互斥,可以看看上面的编译期声明会不会有问题。

噢这样,没具体用过async,感觉怪怪的

你这里的实现编译通不过的,可以试着编译一下,得把模板写到返回值上

伪代码,若要编译如下:

#include <future>
#include <type_traits>

using namespace std;

// launch::async
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::deferred, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::async | launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == static_cast<launch>(launch::async|launch::deferred), int> = 0>
auto async2(_Fp&& __f, _Args&&... __args) {
    return async2<launch::async>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
    // or return async<launch::deferred>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
}

int main() {
    async2<launch::async>([](){});
    async2<launch::deferred>([](){});
    async2<launch::async|launch::deferred>([](){});
}

编过了,我之前以为是

// launch::async
template <launch policy, class _Fp, class... _Args,
    typename = std::enable_if_t<policy == launch::async>>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::deferred
template <launch policy, class _Fp, class... _Args,
    typename = std::enable_if_t<policy == launch::deferred>>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

你这里的实现编译通不过的,可以试着编译一下,得把模板写到返回值上

伪代码,若要编译如下:

#include <future>
#include <type_traits>

using namespace std;

// launch::async
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::deferred, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::async | launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == static_cast<launch>(launch::async|launch::deferred), int> = 0>
auto async2(_Fp&& __f, _Args&&... __args) {
    return async2<launch::async>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
    // or return async<launch::deferred>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
}

int main() {
    async2<launch::async>([](){});
    async2<launch::deferred>([](){});
    async2<launch::async|launch::deferred>([](){});
}

其实可以这样写?

template <launch policy, class Fn, class... Args>
auto async(Fn&& f, Args&&... args) {
    if constexpr ((policy & launch::async) == launch::async) {
        //TODO
    }

    if constexpr ((policy & launch::deferred) == launch::deferred) {
        //TODO
    }
}

template<typename _Fn, typename... _Args>
auto async(_Fn&& __fn, _Args&&... __args) {
      return async<launch::async|launch::deferred>(
        std::forward<_Fn>(__fn),
		std::forward<_Args>(__args)...);
}


int main() {
    async<launch::async>([](){});
    async<launch::deferred>([](){});
    async([](){});
}

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

你这里的实现编译通不过的,可以试着编译一下,得把模板写到返回值上

伪代码,若要编译如下:

#include <future>
#include <type_traits>

using namespace std;

// launch::async
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::deferred, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::async | launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == static_cast<launch>(launch::async|launch::deferred), int> = 0>
auto async2(_Fp&& __f, _Args&&... __args) {
    return async2<launch::async>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
    // or return async<launch::deferred>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
}

int main() {
    async2<launch::async>([](){});
    async2<launch::deferred>([](){});
    async2<launch::async|launch::deferred>([](){});
}

其实可以这样写?

template <launch policy, class Fn, class... Args>
auto async(Fn&& f, Args&&... args) {
    if constexpr ((policy & launch::async) == launch::async) {
        //TODO
    }

    if constexpr ((policy & launch::deferred) == launch::deferred) {
        //TODO
    }
}

template<typename _Fn, typename... _Args>
auto async(_Fn&& __fn, _Args&&... __args) {
      return async<launch::async|launch::deferred>(
        std::forward<_Fn>(__fn),
		std::forward<_Args>(__args)...);
}


int main() {
    async<launch::async>([](){});
    async<launch::deferred>([](){});
    async([](){});
}

其实主要还是返回值和函数调用参数不一样的问题,不然async_simple也能像标准库那样去实现

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 30, 2022

你这里的实现编译通不过的,可以试着编译一下,得把模板写到返回值上

伪代码,若要编译如下:

#include <future>
#include <type_traits>

using namespace std;

// launch::async
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::async, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == launch::deferred, int> = 0>
int async2(_Fp&& __f, _Args&&... __args) {
    // ...
    return 0;
}

// launch::async | launch::deferred
template <launch policy, class _Fp, class... _Args,
    std::enable_if_t<policy == static_cast<launch>(launch::async|launch::deferred), int> = 0>
auto async2(_Fp&& __f, _Args&&... __args) {
    return async2<launch::async>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
    // or return async<launch::deferred>(std::forward<_Fp>(__f), std::forward<_Args>(__args)...);
}

int main() {
    async2<launch::async>([](){});
    async2<launch::deferred>([](){});
    async2<launch::async|launch::deferred>([](){});
}

其实可以这样写?

template <launch policy, class Fn, class... Args>
auto async(Fn&& f, Args&&... args) {
    if constexpr ((policy & launch::async) == launch::async) {
        //TODO
    }

    if constexpr ((policy & launch::deferred) == launch::deferred) {
        //TODO
    }
}

template<typename _Fn, typename... _Args>
auto async(_Fn&& __fn, _Args&&... __args) {
      return async<launch::async|launch::deferred>(
        std::forward<_Fn>(__fn),
		std::forward<_Args>(__args)...);
}


int main() {
    async<launch::async>([](){});
    async<launch::deferred>([](){});
    async([](){});
}

其实主要还是返回值和函数调用参数不一样的问题,不然async_simple也能像标准库那样去实现

暂时只想到将Exector提前这么一个办法

template <Launch T, class F, class C = std::nullptr_t>
inline std::conditional_t<T == Launch::Prompt, Uthread, void>
async(Executor* ex, F&& f, C&& c = nullptr) {
    if constexpr (T == Launch::Schedule) {
        if (!ex)
            AS_UNLIKELY { return; }
        ex->schedule([f = std::move(f), c = std::move(c), ex]() {
            Uthread uth(Attribute{ex}, std::move(f));
            if (c) {
                uth.join(std::move(c));
            } else {
                uth.detach();
            }
        });
    } else if constexpr (T == Launch::Prompt) {
        return Uthread(Attribute{ex}, std::forward<F>(f)); 
    } else if constexpr (T == Launch::Current) {
        Uthread uth(Attribute{ex}, std::forward<F>(f));
        uth.detach();
    } else {
        // Invalid Launch
    }
    
}

Copy link
Collaborator

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

当前的接口设计确实不那么连贯,不过从解决眼下问题的角度以及保持接口的角度出发,我觉得现在这个 patch 也可以接受的

T>::type* = nullptr>
inline Uthread async(F&& f, Executor* ex) {
template <Launch T, class F>
inline std::enable_if_t<T == Launch::Prompt, Uthread> async(F&& f,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些 enable if 改成 requires 会好一些

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这些 enable if 改成 requires 会好一些

OK

@RainMark
Copy link
Collaborator

当时确实主要是返回值和参数类型不一样,不然是可以把policy放到参数上的。考虑到换个函数名字给使用者带来不便利性所以先用模板参数来区分了。

@RainMark
Copy link
Collaborator

感觉是不是可以删掉Prompt那种?而且Prompt貌似更符合sync execution语义

@4kangjc
Copy link
Collaborator Author

4kangjc commented Oct 31, 2022

Why

#171

What is changing

修改了ReadFile.bench.cpp下无符号整数与有符号整数比较产生的编译错误 重构uthread::Launch,修改为enum class

Example

#171

我发现clang下不会触发报错,难怪,你们用的是clang
Screenshot_20221031_220044

@ChuanqiXu9
Copy link
Collaborator

触发报错,难怪,你们用的

对,像这种不同的修改最好放到不同的 PR里比较好

Copy link
Collaborator

@ChuanqiXu9 ChuanqiXu9 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这个 Patch 本身应该是好的;接口设计属于历史遗留问题了

@ChuanqiXu9 ChuanqiXu9 merged commit 789a3e0 into alibaba:main Nov 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants