-
Notifications
You must be signed in to change notification settings - Fork 266
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
Comments
I suggest change IOExecutor to another name. IOExecutor is not derived from Executor, this name may be confusing sometimes. |
Let the user implement IOCallback? |
// 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; |
Maybe |
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 |
// 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 struct IOCallback {
void operator()(io_event_t) {}
void operator()(std::size_t) {}
void operator()(asio::error_code ec, std::size_t) {}
}; |
If IOCallback is incomplete type, the users are hard to know how to call
|
But |
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 |
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. |
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<...>
}; |
hmm, OK |
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. |
这里应该是期望提供基于回调并且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 |
need abstract out a file handle class. |
emmm问个问题,为啥async_simple要引入IOExecutor的抽象,看起来并没有其他组件或者特性依赖于IOExecutor。 |
主要是让 IOAwaiter 可以拿到 IO 接口。我们没有开源相关的 IOAwaiter 的实现。 |
哦哦明白了 |
Search before asking
What happened + What you expected to happen
Current IOExecutor has
linux-aio
related interfaceWe 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 IOExecutorAre you willing to submit a PR?
The text was updated successfully, but these errors were encountered: