-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Issue #506: task_isolation - rebase feature on the new branch, add unit tests, add Lazy wrapper #676
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
base: master
Are you sure you want to change the base?
Conversation
…, add unit tests, add Lazy wrapper
|
I like this very much. I would be tempted to clean the lazy class by extracting a ValueOrException class. The inling of the placement new and other related machinery next to the taskflow machinery makes understanding either more complex. They can be seperated. I have two before and after examples. Before: https://godbolt.org/z/qsEbf569q After: https://godbolt.org/z/dr175s35q ( Note on Godbolt only taskflow trunk is available so I commented out the code that uses The extraction is a simple class which can be tested in isolation. Perhaps the name of the class should be TryCatch because that is really what it is modelling. // New helper class that encapsulates value or exception handling
template <typename T>
class ValueOrException {
alignas(std::max(alignof(T), alignof(std::exception_ptr)))
unsigned char m_data[std::max(sizeof(T), sizeof(std::exception_ptr))];
enum class State { NONE, VALUE, EXCEPTION } m_state = State::NONE;
public:
~ValueOrException() {
reset();
}
/// Emplaces the result of calling fn directly into
/// the store. If the function fails then the exception
/// is stored.
void emplace_fn(std::function<T()> fn) {
try {
new (m_data) T(fn());
m_state = State::VALUE;
} catch (...) {
new (m_data) std::exception_ptr(std::current_exception());
m_state = State::EXCEPTION;
}
}
T* get() {
if (m_state == State::EXCEPTION) {
std::rethrow_exception(*std::launder(reinterpret_cast<std::exception_ptr*>(m_data)));
}
return std::launder(reinterpret_cast<T*>(m_data));
}
bool has_value() const {
return m_state == State::VALUE;
}
bool ready() const {
return m_state != State::NONE;
}
void reset() {
if (m_state == State::VALUE) {
std::launder(reinterpret_cast<T*>(m_data))->~T();
} else if (m_state == State::EXCEPTION) {
std::launder(reinterpret_cast<std::exception_ptr*>(m_data))->~exception_ptr();
}
m_state = State::NONE;
}
}; |
|
Thank you very much for this pull request - let me look into this and see how we can make it better! |
|
Not sure about the implementation, but regarding API: This way user can implement some kind of cooperative mutex locking that does not block the calling thread and instead runs corun_until until mutex is available. |
|
@olologin , I am looking into this pull - and thank you again for this great contribution. A few suggestions:
Feel free to let me know if you have any thoughts. Thank you very much for this pull again! |
Solves #506
Of course I just show my changes, and I expect some improvements must be made to upstream this. Let me know if you see some area for improvement.
Also, I had to rebase these changes from last release branch, so maybe I missed something. But for our internal usage (which is mostly Lazy class) it works fine. We did not see significant slowdowns because of additional operations in scheduler yet.
Would be great to upstream this eventually.
Brief explanation of how this works: #506 (comment)