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

Refactoring IOExecutor #62

Open
2 tasks
RainMark opened this issue Mar 14, 2022 · 22 comments
Open
2 tasks

Refactoring IOExecutor #62

RainMark opened this issue Mar 14, 2022 · 22 comments

Comments

@RainMark
Copy link
Collaborator

RainMark commented Mar 14, 2022

Search before asking

  • I searched the issues and found no similar issues.

What happened + What you expected to happen

Current IOExecutor has linux-aio related interface
We hope to remove platform level api at IOExecutor base class, make IOExecutor become empty class
And we can implement UringExecutor, AsioExecutor, and the other user defined IOExecutor based on IOExecutor

Are you willing to submit a PR?

  • Yes I am willing to submit a PR!
@foreverhy
Copy link
Collaborator

I suggest change IOExecutor to another name. IOExecutor is not derived from Executor, this name may be confusing sometimes.

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 5, 2022

Let the user implement IOCallback?

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 5, 2022

// IO type
using IOOPCode = unsigned int;

// [Require] User implements an IOCallback
struct IOCallback;

    virtual void submitIO(int fd, IOOPCode cmd, void* buffer, size_t length,
                          off_t offset, IOCallback cbfn) = 0;
    virtual void submitIOV(int fd, IOOPCode cmd, const iovec_t* iov,
                           size_t count, off_t offset, IOCallback cbfn) = 0;

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 5, 2022

Maybe fd is also needed, because asio uses Socket

@ChuanqiXu9
Copy link
Collaborator

Let the user implement IOCallback?

Yeah, but the problem is how to design the interface... we feel like the previous design is not generic enough..

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 6, 2022

Let the user implement IOCallback?

Yeah, but the problem is how to design the interface... we feel like the previous design is not generic enough..

No, I mean design IOCallback as an incomplete type and let users implement it themselves

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 6, 2022

// AIOExecutor.cpp
struct IOCallback {
    void operator()(io_event_t) {}
};
// UringExecutor.cpp
struct IOCallback {
   void operator()(std::size_t) {}
};
// ASIOExecutor.cpp
struct IOCallback {
    void operator()(asio::error_code ec, std::size_t) {}
};

Or users need to implement UringExecutor, AsioExecutor, AIOExecutor at the same time

struct IOCallback {
    void operator()(io_event_t) {}
    void operator()(std::size_t) {}
    void operator()(asio::error_code ec, std::size_t) {}
};

@ChuanqiXu9
Copy link
Collaborator

If IOCallback is incomplete type, the users are hard to know how to call submitIO. I think there is only 2 choices:

  1. Design a generic IO interface.
  2. Remove the concept IOExecutor from async_simple, let users to define it for themselves.

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 6, 2022

If IOCallback is incomplete type, the users are hard to know how to call submitIO. I think there is only 2 choices:

  1. Design a generic IO interface.
  2. Remove the concept IOExecutor from async_simple, let users to define it for themselves.

But submitIO is implemented by users themselves,Why is it hard for users to know how to call submitIO?

@ChuanqiXu9
Copy link
Collaborator

If IOCallback is incomplete type, the users are hard to know how to call submitIO. I think there is only 2 choices:

  1. Design a generic IO interface.
  2. Remove the concept IOExecutor from async_simple, let users to define it for themselves.

But submitIO is implemented by users themselves,Why is it hard for users to know how to call submitIO?

In this manner, the call to submitIO can't work if we replace an IOExecutor with another IOExecutor. So it is a fake polymorphism. In this case, it would be better to remove the concept IOExecutor from async_simple. Otherwise, it is bad that the submitIO looks like a polymorphism API while it is not actually.

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 6, 2022

If IOCallback is incomplete type, the users are hard to know how to call submitIO. I think there is only 2 choices:

  1. Design a generic IO interface.
  2. Remove the concept IOExecutor from async_simple, let users to define it for themselves.

But submitIO is implemented by users themselves,Why is it hard for users to know how to call submitIO?

In this manner, the call to submitIO can't work if we replace an IOExecutor with another IOExecutor. So it is a fake polymorphism. In this case, it would be better to remove the concept IOExecutor from async_simple. Otherwise, it is bad that the submitIO looks like a polymorphism API while it is not actually.

I see. You are right.

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 6, 2022

If IOCallback is incomplete type, the users are hard to know how to call submitIO. I think there is only 2 choices:

  1. Design a generic IO interface.
  2. Remove the concept IOExecutor from async_simple, let users to define it for themselves.

But submitIO is implemented by users themselves,Why is it hard for users to know how to call submitIO?

In this manner, the call to submitIO can't work if we replace an IOExecutor with another IOExecutor. So it is a fake polymorphism. In this case, it would be better to remove the concept IOExecutor from async_simple. Otherwise, it is bad that the submitIO looks like a polymorphism API while it is not actually.

I see. You are right.

But, In the fact, It can work if we replace an InExecutor with another IOExecutor

@ChuanqiXu9
Copy link
Collaborator

If IOCallback is incomplete type, the users are hard to know how to call submitIO. I think there is only 2 choices:

  1. Design a generic IO interface.
  2. Remove the concept IOExecutor from async_simple, let users to define it for themselves.

But submitIO is implemented by users themselves,Why is it hard for users to know how to call submitIO?

In this manner, the call to submitIO can't work if we replace an IOExecutor with another IOExecutor. So it is a fake polymorphism. In this case, it would be better to remove the concept IOExecutor from async_simple. Otherwise, it is bad that the submitIO looks like a polymorphism API while it is not actually.

I see. You are right.

But, In the fact, It can work if we replace an InExecutor with another IOExecutor

No, it can't since the definition of IOCallback is missing in async_simple and we won't have a semantic constraint for it in this case. For example, if one implementation requires IOCallback to receive 2 arguments and another one requires it to receive 1 argument, it is clearly that they are not interchangeable.

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 6, 2022

If IOCallback is incomplete type, the users are hard to know how to call submitIO. I think there is only 2 choices:

  1. Design a generic IO interface.
  2. Remove the concept IOExecutor from async_simple, let users to define it for themselves.

But submitIO is implemented by users themselves,Why is it hard for users to know how to call submitIO?

In this manner, the call to submitIO can't work if we replace an IOExecutor with another IOExecutor. So it is a fake polymorphism. In this case, it would be better to remove the concept IOExecutor from async_simple. Otherwise, it is bad that the submitIO looks like a polymorphism API while it is not actually.

I see. You are right.

But, In the fact, It can work if we replace an InExecutor with another IOExecutor

Function overloading can be achieved

struct IOCallback {
    IOCallback(AIOCallback) {}
    IOCallback(AsioCallback) {}
    IOCallback(UringCallback) {}
    void operator()(io_event_t) {}
    void operator()(std::size_t) {}
    void operator()(asio::error_code ec, std::size_t) {}
private:
    std::variant<...> 
};

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 6, 2022

hmm, OK

@ChuanqiXu9
Copy link
Collaborator

struct IOCallback {
    IOCallback(AIOCallback) {}
    IOCallback(AsioCallback) {}
    IOCallback(UringCallback) {}
    void operator()(io_event_t) {}
    void operator()(std::size_t) {}
    void operator()(asio::error_code ec, std::size_t) {}
private:
    std::variant<...> 
};

We can't assume the implementation of IOCallback in this case. Also we can't assume there would only be 3 IOExecutor in this case.

@RainMark
Copy link
Collaborator Author

RainMark commented Dec 7, 2022

这里应该是期望提供基于回调并且posix like的接口,例如

class IOEngine {
  using Callback = std::function<void(int)>;

  int read(char* buf, size_t size, Callback&& cb);
  int pread(char* buf, size_t size, size_t off,  Callback&& cb);
  // ...
};

不要让用户看到很裸的aio那种接口去用了

@4kangjc
Copy link
Collaborator

4kangjc commented Dec 8, 2022

这里应该是期望提供基于回调并且posix like的接口,例如

class IOEngine {
  using Callback = std::function<void(int)>;

  int read(char* buf, size_t size, Callback&& cb);
  int pread(char* buf, size_t size, size_t off,  Callback&& cb);
  // ...
};

不要让用户看到很裸的aio那种接口去用了

The trouble is that many IO libraries will not use fd directly, such as asio. It is difficult to design a generic IO interface.

@RainMark
Copy link
Collaborator Author

RainMark commented Dec 8, 2022

这里应该是期望提供基于回调并且posix like的接口,例如

class IOEngine {
  using Callback = std::function<void(int)>;

  int read(char* buf, size_t size, Callback&& cb);
  int pread(char* buf, size_t size, size_t off,  Callback&& cb);
  // ...
};

不要让用户看到很裸的aio那种接口去用了

The trouble is that many IO libraries will not use fd directly, such as asio. It is difficult to design a generic IO interface.

need abstract out a file handle class.

@chloro-pn
Copy link
Collaborator

emmm问个问题,为啥async_simple要引入IOExecutor的抽象,看起来并没有其他组件或者特性依赖于IOExecutor。

@ChuanqiXu9
Copy link
Collaborator

主要是让 IOAwaiter 可以拿到 IO 接口。我们没有开源相关的 IOAwaiter 的实现。

@chloro-pn
Copy link
Collaborator

哦哦明白了

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

No branches or pull requests

5 participants