-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Exposing Channel to be used as a Variable and integrating with Fluid #8486
Conversation
Add ChannelHolder to Channel
if (channel_) channel_->Close(); | ||
} | ||
|
||
std::unique_ptr<Channel<T>*> channel_; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks to me that it might be possible to remove the current base class Channel
from channel.h
, and rename ChannelHolder
into Channel
, if we write here
std::unique_ptr<
boost::variant<
BufferedChannel<T>,
UnbufferedChannel<T>>> channel_;
I am not an expert of boost::variant
, but I see it is used many places in our codebase. For example:
Paddle/paddle/fluid/platform/place.h
Line 53 in 24509f4
typedef boost::variant<CUDAPlace, CPUPlace> Place; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I verified that we could use the polymorphism provided by boost::variant to call Send:
#include <iostream>
#include <boost/variant.hpp>
class BufferedChannel {
public:
bool Send() {
std::cout << "BufferedChannel::Send\n";
return false;
}
};
class UnbufferedChannel {
public:
bool Send() {
std::cout << "UnbufferedChannel::Send\n";
return false;
}
};
class SendVisitor : public boost::static_visitor<bool> {
public:
template <typename T>
bool operator()(T& t) const {
return t.Send();
}
};
int main() {
boost::variant<BufferedChannel, UnbufferedChannel> ch;
BufferedChannel bch;
ch = bch;
boost::apply_visitor(SendVisitor(), ch);
return 0;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you @wangkuiyi . This looks interesting. I'll look into this Visitor pattern polymorphism.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Extend the above example to work with unique_ptr:
#include <iostream>
#include <boost/variant.hpp>
class BufferedChannel {
public:
bool Send() {
std::cout << "BufferedChannel::Send\n";
return false;
}
};
class UnbufferedChannel {
public:
bool Send() {
std::cout << "UnbufferedChannel::Send\n";
return false;
}
};
class SendVisitor : public boost::static_visitor<bool> {
public:
template <typename T>
bool operator()(T* t) const {
return t->Send();
}
};
int main() {
typedef boost::variant<BufferedChannel*, UnbufferedChannel*> ChannelVariant;
std::unique_ptr<ChannelVariant> ch;
ch.reset(new ChannelVariant(new BufferedChannel));
boost::apply_visitor(SendVisitor(), *ch.get());
return 0;
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am open to merge this PR and then try to remove a level of abstraction -- remove class Channel
or remove class ChannelHolder
.
Thank you so much for your comments @wangkuiyi , we are looking into doing it the |
Fixes : #8485 and #8484
@abhinavarora and I have been pair programming to expose the Channel class to be used a Variable and integrating it with the rest of Fluid framework.