You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2022/02/13 06:12:18 UTC
[GitHub] [tvm] huajsj opened a new pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
huajsj opened a new pull request #10234:
URL: https://github.com/apache/tvm/pull/10234
In this patch we create multiple working threads for each runtime of pipeline. the threads would be terminated once the runtime class gets destroyed.
We also add a notification mechanism derived from the 'binding configuration' of the runtime to forward the data notification.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#issuecomment-1039411862
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r810788822
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
+ Returns
+ -------
+ count : int
+ The times of pipeline running.
+ """
+ return self._get_pipe_execute_number()
Review comment:
I think `num_executing_pipeline` is better.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi merged pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi merged pull request #10234:
URL: https://github.com/apache/tvm/pull/10234
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#issuecomment-1039884305
Thanks @huajsj. This is interesting and tricky code, I'll try review carefully.
Where does this PR sit in the feature road map?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r810791253
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
Review comment:
You already use many `*.first, *.second` etc and it is very hard to understand what they represent. So I please come up with more descriptive names, rather than introducing another `first, second` mess.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r813343920
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +53,99 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the runtime module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The runtime module interface type.*/
+enum InterfaceType {
+ INPUT = 0,
+ OUTPUT,
+};
+/*!
+ *\brief The structure includes the module index and the module output index.
+ */
+struct ModuleInterfaceID {
+ ModuleInterfaceID() : runtime_idx(0), runtime_interface_idx(0), interface_type(OUTPUT) { ; }
+ ModuleInterfaceID(int runtime_index, int runtime_interface_index, InterfaceType type = OUTPUT) {
+ runtime_idx = runtime_index;
+ runtime_interface_idx = runtime_interface_index;
+ interface_type = type;
+ }
+ int runtime_idx;
+ union {
+ /*!\brief The output interface index.*/
+ int runtime_output_idx;
+ /*!\brief The input interface index.*/
+ int runtime_input_idx;
+ /*!\brief The interface index.*/
+ int runtime_interface_idx;
+ };
+ /*!\brief The interface type*/
+ InterfaceType interface_type;
+};
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable notify_cv;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ std::atomic<bool> exit_state_{false};
+ /*!
+ * \brief The 'ModuleInterfaceID' in which the data was ready and triggered this
+ * notification.
+ */
+ ModuleInterfaceID notification_source_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_output_id The id of a runtime interface which is sending out the data
+ * notification.
+ */
+ explicit DataNotify(ModuleInterfaceID parent_output_id) {
+ notification_source_ = parent_output_id;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ ModuleInterfaceID GetNotifySource(void) { return notification_source_; }
+ /*!
+ *\brief Waiting for the notification.
+ *\return Returning the value 'false' when the notification is in a 'exit' state, else
+ * return true.
+ */
+ bool Wait(void) {
+ std::unique_lock<std::mutex> lock(mutex_);
+ notify_cv.wait(lock, [&] { return this->data_ready_; });
+ data_ready_ = false;
+ return !exit_state_;
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812646438
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
@masahi ,@junrushao1994 , thanks for the follow up, yes I agree with that std::atomic should be the correct solution to guarantee MT instead of using 'volatile' which may still need a memory barrier solution.
the latest change removed volatile for data_ready_ because mutex already did the acquire/release memory barrier, and change exit_state_ into a std::atomic<bool>, please check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#issuecomment-1039901920
> Thanks @huajsj. This is interesting and tricky code, I'll try review carefully. cc @comaniac
>
> Where does this PR sit in the feature road map?
Thanks @masahi , this PR is part of RFC https://github.com/apache/tvm-rfcs/blob/main/rfcs/0014-pipeline-executor.md, the tracking issue is https://github.com/apache/tvm/issues/8596.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r806618932
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +477,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> childs_;
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +477,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> childs_;
+ /*\brief A map including the runtime input index and the notification data struction.*/
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -386,6 +506,99 @@ class BackendRuntime {
tvm::runtime::PackedFunc get_num_inputs_;
tvm::runtime::PackedFunc get_input_index_;
tvm::runtime::PackedFunc run_;
+ /*!\brief The working thread is used to execute the runtimes in pipeline.*/
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r806551025
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
seems like not need to use atomic, because 'data_ready_' already protected by 'mutex_' and for exit_state, there is only a single 'writing' in ExitNotify then no race condition can happen. although atomic is very light weight but it still involved additional perf cost, if not necessary we may need to avoid use it here.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_idx The index of runtime which is sending out the data notification
+ * \param parent_output_idx The index of runtime output interface which is sending out
+ * the data notification.
+ */
+ DataNotify(int parent_idx, int parent_output_idx) {
+ parent_idx_ = parent_idx;
+ parent_output_idx_ = parent_output_idx;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ std::pair<int, int> GetNotifySource(void) {
+ return std::make_pair(parent_idx_, parent_output_idx_);
+ }
+ /*!
+ *\brief Waiting the notification.
+ *\return Returning the value 'false' when the notification is in a 'exit' state, else
+ * return true.
+ */
+ bool Wait(void) {
+ std::unique_lock<std::mutex> lock(mutex_);
+ condition_.wait(lock, [&] { return this->data_ready_; });
+ data_ready_ = false;
+ return !exit_state_;
+ }
+ /*!brief Sending the notification in which the related data is ready.*/
+ void Notify(void) {
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ data_ready_ = true;
Review comment:
std::contiditon_variable already used a mutex to protect data_ready_, to cooperate with such mechanism we may need use mutex at this function too.
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
+ Returns
+ -------
+ count : int
+ The times of pipeline running.
+ """
+ return self._get_pipe_execute_number()
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_executor.cc
##########
@@ -242,6 +237,13 @@ std::pair<int, int> PipelineExecutor::GetInputIndex(const std::string& name) {
auto gruntime = runtimes_[index.first];
return std::make_pair(index.first, gruntime->GetInputIndex(index.second));
}
+/*!
+ * \brief Getting the times of pipeline running.
+ * \return Returning the times of pipeline running.
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_scheduler.cc
##########
@@ -46,6 +47,10 @@ std::vector<std::shared_ptr<BackendRuntime>> PipelineScheduler::PipelineInit(
NDArray output = runtimes[output_pair.first]->CreateFromOutput(output_pair.second);
output_arrays_.push_back(output);
}
+ // Initializing and then running the work thread.
+ for (auto runtime : runtimes) {
+ runtime->Pipeline_Initialize(pipeline_config, &runtimes);
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_idx The index of runtime which is sending out the data notification
+ * \param parent_output_idx The index of runtime output interface which is sending out
+ * the data notification.
+ */
+ DataNotify(int parent_idx, int parent_output_idx) {
+ parent_idx_ = parent_idx;
+ parent_output_idx_ = parent_output_idx;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ std::pair<int, int> GetNotifySource(void) {
+ return std::make_pair(parent_idx_, parent_output_idx_);
+ }
+ /*!
+ *\brief Waiting the notification.
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -53,6 +132,16 @@ class ConfigBindings {
int GetGlobalOutputIndex() const { return global_output_index_; }
/*!\brief Returning the binding configuration.*/
std::unordered_map<int, std::string>& Get() { return bindings_; }
+ /*!
+ * \brief Enumrating the binding configuration.
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -189,6 +287,18 @@ class ConfigPipelineExecution {
ICHECK(config_.find(key) != config_.end());
return config_[key];
}
+ /*!
+ * \brief Enumurating the binding configuration for a specify runtime.
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +477,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> childs_;
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +477,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> childs_;
+ /*\brief A map including the runtime input index and the notification data struction.*/
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -386,6 +506,99 @@ class BackendRuntime {
tvm::runtime::PackedFunc get_num_inputs_;
tvm::runtime::PackedFunc get_input_index_;
tvm::runtime::PackedFunc run_;
+ /*!\brief The working thread is used to execute the runtimes in pipeline.*/
Review comment:
fixed.
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
Review comment:
the first one is the runtime index, second one is the output index in the runtime, we use such 2 index value to located a output data in pipeline forwarding.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
Review comment:
the first one is the runtime index, second one is the output index in the runtime, we use such 2 index value to located a output data in pipeline forwarding. merged these 2 value into a ModuleOutputPair structure to make it more readable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r806466228
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
Review comment:
running pipeline
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r806591088
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_idx The index of runtime which is sending out the data notification
+ * \param parent_output_idx The index of runtime output interface which is sending out
+ * the data notification.
+ */
+ DataNotify(int parent_idx, int parent_output_idx) {
+ parent_idx_ = parent_idx;
+ parent_output_idx_ = parent_output_idx;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ std::pair<int, int> GetNotifySource(void) {
+ return std::make_pair(parent_idx_, parent_output_idx_);
+ }
+ /*!
+ *\brief Waiting the notification.
+ *\return Returning the value 'false' when the notification is in a 'exit' state, else
+ * return true.
+ */
+ bool Wait(void) {
+ std::unique_lock<std::mutex> lock(mutex_);
+ condition_.wait(lock, [&] { return this->data_ready_; });
+ data_ready_ = false;
+ return !exit_state_;
+ }
+ /*!brief Sending the notification in which the related data is ready.*/
+ void Notify(void) {
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ data_ready_ = true;
Review comment:
std::contiditon_variable already used a mutex to protect data_ready_, to cooperate with such mechanism we may need use mutex at this function too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r807155373
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812662433
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
+ Returns
+ -------
+ count : int
+ The times of pipeline running.
+ """
+ return self._get_pipe_execute_number()
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r814190841
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +502,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> children_;
+ /*\brief A map including the runtime input index and the notification data structure.*/
+ std::unordered_map<int, std::shared_ptr<DataNotify>> parents_notify_;
+ /*\brief The times of using pipeline function. */
+ uint32_t statistic_pipeline_execute_times_ = 0;
Review comment:
sure, will do that, thanks @masahi.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi edited a comment on pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi edited a comment on pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#issuecomment-1039884305
Thanks @huajsj. This is interesting and tricky code, I'll try review carefully. cc @comaniac
Where does this PR sit in the feature road map?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812441417
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
I agree with that volatile is bad practice for 'atomic' guarantee, but in our case we only need to guarantee the data 'visibility' under multiple thread mode , for such requirement volatile should be ok.
about 'atomic' guarantee part, for vairable 'data_ready_' the existing 'mutex_' and conditional variable will help for the atomic guarantee, for variable exit_state_, because each notification only have one parent and one child, under such single consumer and single producer scenario the both reading and writing of 'exit_state' are native atomic then no need additional protection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812661375
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
@masahi ,@junrushao1994, @areusch , thanks for the follow up, I agree with that the promise is a better primitive and memory fence also be another issue we need to correct handle, after review code, I just realized there is still a memory barrier need to put after set 'exit_state = true' and before 'notify()' to guarantee the code 'ordering' , for such issue I may either use std::atomic or use 'promise + release barrier' and seems like std::atomic should be the better solution with one logic.
in the latest change I removed volatile for data_ready_ because mutex already did the acquire/release memory barrier, and change exit_state_ into a std::atomic<bool>, please check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r813333807
##########
File path: src/runtime/pipeline/pipeline_scheduler.cc
##########
@@ -32,6 +32,7 @@ std::vector<std::shared_ptr<BackendRuntime>> PipelineScheduler::PipelineInit(
const std::vector<Module>& modules, const ConfigPipelineExecution& pipeline_config) {
std::vector<std::shared_ptr<BackendRuntime>> runtimes;
graph_modules_ = modules;
+ // Creating a list of runtimes.
for (size_t i = 0; i < graph_modules_.size(); i++) {
auto runItem = std::make_shared<BackendRuntime>(graph_modules_[i], i);
runtimes.push_back(runItem);
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r807158199
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
Review comment:
the first one is the runtime index, second one is the output index in the runtime, we use such 2 index value to located a output data in pipeline forwarding.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#issuecomment-1039884305
Thanks @huajsj. This is interesting and tricky code, I'll try review carefully.
Where does this PR sit in the feature road map?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812550486
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
I don't know, but all of SO answers I looked at say volatile is not needed for threading purpose. For example https://stackoverflow.com/questions/4557979/when-to-use-volatile-with-multi-threading
Any thought?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r813334713
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +53,99 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the runtime module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The runtime module interface type.*/
+enum InterfaceType {
+ INPUT = 0,
+ OUTPUT,
+};
+/*!
+ *\brief The structure includes the module index and the module output index.
+ */
+struct ModuleInterfaceID {
+ ModuleInterfaceID() : runtime_idx(0), runtime_interface_idx(0), interface_type(OUTPUT) { ; }
+ ModuleInterfaceID(int runtime_index, int runtime_interface_index, InterfaceType type = OUTPUT) {
+ runtime_idx = runtime_index;
+ runtime_interface_idx = runtime_interface_index;
+ interface_type = type;
+ }
+ int runtime_idx;
+ union {
+ /*!\brief The output interface index.*/
+ int runtime_output_idx;
+ /*!\brief The input interface index.*/
+ int runtime_input_idx;
+ /*!\brief The interface index.*/
+ int runtime_interface_idx;
+ };
+ /*!\brief The interface type*/
+ InterfaceType interface_type;
+};
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable notify_cv;
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r813334515
##########
File path: src/runtime/pipeline/pipeline_executor.h
##########
@@ -95,6 +95,10 @@ class TVM_DLL PipelineExecutor : public ModuleNode {
* \return Return input data.
*/
NDArray GetInput(std::string input_name);
+ /*!
+ * \brief Getting the count of running pipeline.
+ */
+ int PipelineStatisticPipeExecuterCount();
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#issuecomment-1043591966
@masahi, all review comments get addressed, please take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r813778207
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +502,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> children_;
+ /*\brief A map including the runtime input index and the notification data structure.*/
+ std::unordered_map<int, std::shared_ptr<DataNotify>> parents_notify_;
+ /*\brief The times of using pipeline function. */
+ uint32_t statistic_pipeline_execute_times_ = 0;
Review comment:
This variable names and its doc does not make a lot of sense ("the times of ..." is not really a word). In the next PR please improve them.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] junrushao1994 commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
junrushao1994 commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812554431
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
volatile has nothing to do with multi-threading or memory fence. it's only used in modern c++ to enforce reading from memory and disallow compilers to put the specific data from the register. Note that no memory fence is required to be introduced by volatile, so it doesn't officially guarantee anything about visibility. The rule of thumb is to never use volatile unless for usecases like DMA
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812646779
##########
File path: src/runtime/pipeline/pipeline_executor.cc
##########
@@ -79,8 +79,10 @@ PackedFunc PipelineExecutor::GetFunction(const std::string& name,
[sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->GetOutput(); });
} else if (name == "run") {
return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { this->Run(args[0]); });
- } else if (name == "stop") {
- return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { this->Stop(); });
+ } else if (name == "get_statistic_pipe_execute_number") {
+ return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) {
+ *rv = this->PipelineStatisticPipeExecuteNumber();
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] areusch commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
areusch commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812580805
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
@huajsj what do you think about using something like `std::promise` instead? while I think the use of volatile here is probably okay in a consumer-producer relation, i'm wondering if we can use a more established primitive to enable more folks in the community to work on the code.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi edited a comment on pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi edited a comment on pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#issuecomment-1039884305
Thanks @huajsj. This is interesting and tricky code, I'll try review carefully. cc @comaniac
Where does this PR sit in the feature road map?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r806618058
##########
File path: src/runtime/pipeline/pipeline_executor.cc
##########
@@ -242,6 +237,13 @@ std::pair<int, int> PipelineExecutor::GetInputIndex(const std::string& name) {
auto gruntime = runtimes_[index.first];
return std::make_pair(index.first, gruntime->GetInputIndex(index.second));
}
+/*!
+ * \brief Getting the times of pipeline running.
+ * \return Returning the times of pipeline running.
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_scheduler.cc
##########
@@ -46,6 +47,10 @@ std::vector<std::shared_ptr<BackendRuntime>> PipelineScheduler::PipelineInit(
NDArray output = runtimes[output_pair.first]->CreateFromOutput(output_pair.second);
output_arrays_.push_back(output);
}
+ // Initializing and then running the work thread.
+ for (auto runtime : runtimes) {
+ runtime->Pipeline_Initialize(pipeline_config, &runtimes);
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_idx The index of runtime which is sending out the data notification
+ * \param parent_output_idx The index of runtime output interface which is sending out
+ * the data notification.
+ */
+ DataNotify(int parent_idx, int parent_output_idx) {
+ parent_idx_ = parent_idx;
+ parent_output_idx_ = parent_output_idx;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ std::pair<int, int> GetNotifySource(void) {
+ return std::make_pair(parent_idx_, parent_output_idx_);
+ }
+ /*!
+ *\brief Waiting the notification.
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -53,6 +132,16 @@ class ConfigBindings {
int GetGlobalOutputIndex() const { return global_output_index_; }
/*!\brief Returning the binding configuration.*/
std::unordered_map<int, std::string>& Get() { return bindings_; }
+ /*!
+ * \brief Enumrating the binding configuration.
Review comment:
fixed.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -189,6 +287,18 @@ class ConfigPipelineExecution {
ICHECK(config_.find(key) != config_.end());
return config_[key];
}
+ /*!
+ * \brief Enumurating the binding configuration for a specify runtime.
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r806617927
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
+ Returns
+ -------
+ count : int
+ The times of pipeline running.
+ """
+ return self._get_pipe_execute_number()
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#issuecomment-1039411862
@comaniac , @masahi , please take a look.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r806466392
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
+ Returns
+ -------
+ count : int
+ The times of pipeline running.
+ """
+ return self._get_pipe_execute_number()
Review comment:
You are using the term `time` or `number` throughout the code, but please use more specific terms. I think `time` is more like `duration` and `number` is more like `elapsed_time`?
Please update your naming convention in all files. I won't point out each of them.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
Use atomic
##########
File path: src/runtime/pipeline/pipeline_executor.cc
##########
@@ -242,6 +237,13 @@ std::pair<int, int> PipelineExecutor::GetInputIndex(const std::string& name) {
auto gruntime = runtimes_[index.first];
return std::make_pair(index.first, gruntime->GetInputIndex(index.second));
}
+/*!
+ * \brief Getting the times of pipeline running.
+ * \return Returning the times of pipeline running.
Review comment:
Not useful to repeat similar comments.
##########
File path: src/runtime/pipeline/pipeline_scheduler.cc
##########
@@ -46,6 +47,10 @@ std::vector<std::shared_ptr<BackendRuntime>> PipelineScheduler::PipelineInit(
NDArray output = runtimes[output_pair.first]->CreateFromOutput(output_pair.second);
output_arrays_.push_back(output);
}
+ // Initializing and then running the work thread.
+ for (auto runtime : runtimes) {
+ runtime->Pipeline_Initialize(pipeline_config, &runtimes);
Review comment:
InitializePipeline
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
Review comment:
A better name than `condition`? How about `notify_cv`.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_idx The index of runtime which is sending out the data notification
+ * \param parent_output_idx The index of runtime output interface which is sending out
+ * the data notification.
+ */
+ DataNotify(int parent_idx, int parent_output_idx) {
+ parent_idx_ = parent_idx;
+ parent_output_idx_ = parent_output_idx;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ std::pair<int, int> GetNotifySource(void) {
+ return std::make_pair(parent_idx_, parent_output_idx_);
+ }
+ /*!
+ *\brief Waiting the notification.
+ *\return Returning the value 'false' when the notification is in a 'exit' state, else
+ * return true.
+ */
+ bool Wait(void) {
+ std::unique_lock<std::mutex> lock(mutex_);
+ condition_.wait(lock, [&] { return this->data_ready_; });
+ data_ready_ = false;
+ return !exit_state_;
+ }
+ /*!brief Sending the notification in which the related data is ready.*/
+ void Notify(void) {
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ data_ready_ = true;
Review comment:
Can get rid of mutex if you make `data_ready` atomic
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
Review comment:
I don't understand the difference of the two.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -386,6 +506,99 @@ class BackendRuntime {
tvm::runtime::PackedFunc get_num_inputs_;
tvm::runtime::PackedFunc get_input_index_;
tvm::runtime::PackedFunc run_;
+ /*!\brief The working thread is used to execute the runtimes in pipeline.*/
Review comment:
worker
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -189,6 +287,18 @@ class ConfigPipelineExecution {
ICHECK(config_.find(key) != config_.end());
return config_[key];
}
+ /*!
+ * \brief Enumurating the binding configuration for a specify runtime.
Review comment:
typo
specified
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_idx The index of runtime which is sending out the data notification
+ * \param parent_output_idx The index of runtime output interface which is sending out
+ * the data notification.
+ */
+ DataNotify(int parent_idx, int parent_output_idx) {
+ parent_idx_ = parent_idx;
+ parent_output_idx_ = parent_output_idx;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ std::pair<int, int> GetNotifySource(void) {
+ return std::make_pair(parent_idx_, parent_output_idx_);
+ }
+ /*!
+ *\brief Waiting the notification.
Review comment:
Waiting for
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +477,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> childs_;
+ /*\brief A map including the runtime input index and the notification data struction.*/
Review comment:
structure
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +477,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> childs_;
Review comment:
children
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -53,6 +132,16 @@ class ConfigBindings {
int GetGlobalOutputIndex() const { return global_output_index_; }
/*!\brief Returning the binding configuration.*/
std::unordered_map<int, std::string>& Get() { return bindings_; }
+ /*!
+ * \brief Enumrating the binding configuration.
Review comment:
typo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812646438
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
@masahi ,@junrushao1994 , thanks for the follow up, yes I agree with that std::atomic should be the correct solution to guarantee MT instead of using 'volatile' which may still need a memory barrier solution.
the latest change removed volatile for data_ready_ because mutex already did the acquire/release memory barrier, and change exit_state_ into a std::atomic<bool>, please check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r813334040
##########
File path: src/runtime/pipeline/pipeline_scheduler.cc
##########
@@ -101,18 +106,11 @@ void PipelineScheduler::PipelineRunSequential(
void PipelineScheduler::PipelineRun(const std::vector<std::shared_ptr<BackendRuntime>>& runtimes,
ConfigPipelineExecution pipeline_config, bool sequential_mode) {
if (!sequential_mode) {
- // TODO(huajsj) remove this check after all of pipeline features in.
- LOG(FATAL) << "Currently only supports sequential mode.";
+ runtimes.front()->RunPipeLine();
Review comment:
fixed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r807158199
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
Review comment:
the first one is the runtime index, second one is the output index in the runtime, we use such 2 index value to located a output data in pipeline forwarding. merged these 2 value into a ModuleOutputPair structure to make it more readable.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r806551025
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
seems like not need to use atomic, because 'data_ready_' already protected by 'mutex_' and for exit_state, there is only a single 'writing' in ExitNotify then no race condition can happen. although atomic is very light weight but it still involved additional perf cost, if not necessary we may need to avoid use it here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r806466228
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
Review comment:
running pipeline
##########
File path: python/tvm/contrib/pipeline_executor.py
##########
@@ -213,6 +209,16 @@ def get_output(self):
"""
return self._get_output()
+ @property
+ def number_pipe_execute(self):
+ """Getting the times of pipeline running.
+ Returns
+ -------
+ count : int
+ The times of pipeline running.
+ """
+ return self._get_pipe_execute_number()
Review comment:
You are using the term `time` or `number` throughout the code, but please use more specific terms. I think `time` is more like `duration` and `number` is more like `elapsed_time`?
Please update your naming convention in all files. I won't point out each of them.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
Use atomic
##########
File path: src/runtime/pipeline/pipeline_executor.cc
##########
@@ -242,6 +237,13 @@ std::pair<int, int> PipelineExecutor::GetInputIndex(const std::string& name) {
auto gruntime = runtimes_[index.first];
return std::make_pair(index.first, gruntime->GetInputIndex(index.second));
}
+/*!
+ * \brief Getting the times of pipeline running.
+ * \return Returning the times of pipeline running.
Review comment:
Not useful to repeat similar comments.
##########
File path: src/runtime/pipeline/pipeline_scheduler.cc
##########
@@ -46,6 +47,10 @@ std::vector<std::shared_ptr<BackendRuntime>> PipelineScheduler::PipelineInit(
NDArray output = runtimes[output_pair.first]->CreateFromOutput(output_pair.second);
output_arrays_.push_back(output);
}
+ // Initializing and then running the work thread.
+ for (auto runtime : runtimes) {
+ runtime->Pipeline_Initialize(pipeline_config, &runtimes);
Review comment:
InitializePipeline
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
Review comment:
A better name than `condition`? How about `notify_cv`.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_idx The index of runtime which is sending out the data notification
+ * \param parent_output_idx The index of runtime output interface which is sending out
+ * the data notification.
+ */
+ DataNotify(int parent_idx, int parent_output_idx) {
+ parent_idx_ = parent_idx;
+ parent_output_idx_ = parent_output_idx;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ std::pair<int, int> GetNotifySource(void) {
+ return std::make_pair(parent_idx_, parent_output_idx_);
+ }
+ /*!
+ *\brief Waiting the notification.
+ *\return Returning the value 'false' when the notification is in a 'exit' state, else
+ * return true.
+ */
+ bool Wait(void) {
+ std::unique_lock<std::mutex> lock(mutex_);
+ condition_.wait(lock, [&] { return this->data_ready_; });
+ data_ready_ = false;
+ return !exit_state_;
+ }
+ /*!brief Sending the notification in which the related data is ready.*/
+ void Notify(void) {
+ {
+ std::lock_guard<std::mutex> lock(mutex_);
+ data_ready_ = true;
Review comment:
Can get rid of mutex if you make `data_ready` atomic
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
Review comment:
I don't understand the difference of the two.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -386,6 +506,99 @@ class BackendRuntime {
tvm::runtime::PackedFunc get_num_inputs_;
tvm::runtime::PackedFunc get_input_index_;
tvm::runtime::PackedFunc run_;
+ /*!\brief The working thread is used to execute the runtimes in pipeline.*/
Review comment:
worker
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -189,6 +287,18 @@ class ConfigPipelineExecution {
ICHECK(config_.find(key) != config_.end());
return config_[key];
}
+ /*!
+ * \brief Enumurating the binding configuration for a specify runtime.
Review comment:
typo
specified
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_idx The index of runtime which is sending out the data notification
+ * \param parent_output_idx The index of runtime output interface which is sending out
+ * the data notification.
+ */
+ DataNotify(int parent_idx, int parent_output_idx) {
+ parent_idx_ = parent_idx;
+ parent_output_idx_ = parent_output_idx;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ std::pair<int, int> GetNotifySource(void) {
+ return std::make_pair(parent_idx_, parent_output_idx_);
+ }
+ /*!
+ *\brief Waiting the notification.
Review comment:
Waiting for
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +477,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> childs_;
+ /*\brief A map including the runtime input index and the notification data struction.*/
Review comment:
structure
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -367,11 +477,21 @@ struct ParamConnectionConfig {
*\brief Backend Runtime.
*/
class BackendRuntime {
+ using ModuleInputPairList = std::vector<std::pair<std::shared_ptr<BackendRuntime>, int>>;
+
private:
/*\brief The index of runtime indicates the runtime position in the pipeline.*/
int runtime_idx_;
/*\brief The Runtime module of a backend graph executor.*/
Module module_;
+ /*\brief The thread is associated with the current runtime*/
+ std::thread thread_;
+ /*\brief A list of runtime which depends on the current runtime.*/
+ std::unordered_map<int, ModuleInputPairList> childs_;
Review comment:
children
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -53,6 +132,16 @@ class ConfigBindings {
int GetGlobalOutputIndex() const { return global_output_index_; }
/*!\brief Returning the binding configuration.*/
std::unordered_map<int, std::string>& Get() { return bindings_; }
+ /*!
+ * \brief Enumrating the binding configuration.
Review comment:
typo
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] huajsj commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
huajsj commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812662001
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
+ /*!\brief The index of runtime which is sending out the data notification.*/
+ int parent_idx_;
+ /*!\brief The index of runtime output interface which is sending out the data notification.*/
+ int parent_output_idx_;
Review comment:
use the new structure ModuleInterfaceID to make the code more readable, please check.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] leeexyz commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
leeexyz commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r812847282
##########
File path: src/runtime/pipeline/pipeline_executor.h
##########
@@ -95,6 +95,10 @@ class TVM_DLL PipelineExecutor : public ModuleNode {
* \return Return input data.
*/
NDArray GetInput(std::string input_name);
+ /*!
+ * \brief Getting the count of running pipeline.
+ */
+ int PipelineStatisticPipeExecuterCount();
Review comment:
Can we change the function name a little bit shorter? Such as `GetExecutionCount`.
##########
File path: src/runtime/pipeline/pipeline_scheduler.cc
##########
@@ -32,6 +32,7 @@ std::vector<std::shared_ptr<BackendRuntime>> PipelineScheduler::PipelineInit(
const std::vector<Module>& modules, const ConfigPipelineExecution& pipeline_config) {
std::vector<std::shared_ptr<BackendRuntime>> runtimes;
graph_modules_ = modules;
+ // Creating a list of runtimes.
for (size_t i = 0; i < graph_modules_.size(); i++) {
auto runItem = std::make_shared<BackendRuntime>(graph_modules_[i], i);
runtimes.push_back(runItem);
Review comment:
I think it is better to use lower case and underscore, `run_item`.
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +53,99 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the runtime module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The runtime module interface type.*/
+enum InterfaceType {
+ INPUT = 0,
+ OUTPUT,
+};
+/*!
+ *\brief The structure includes the module index and the module output index.
+ */
+struct ModuleInterfaceID {
+ ModuleInterfaceID() : runtime_idx(0), runtime_interface_idx(0), interface_type(OUTPUT) { ; }
+ ModuleInterfaceID(int runtime_index, int runtime_interface_index, InterfaceType type = OUTPUT) {
+ runtime_idx = runtime_index;
+ runtime_interface_idx = runtime_interface_index;
+ interface_type = type;
+ }
+ int runtime_idx;
+ union {
+ /*!\brief The output interface index.*/
+ int runtime_output_idx;
+ /*!\brief The input interface index.*/
+ int runtime_input_idx;
+ /*!\brief The interface index.*/
+ int runtime_interface_idx;
+ };
+ /*!\brief The interface type*/
+ InterfaceType interface_type;
+};
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable notify_cv;
Review comment:
change `notify_cv` to `notify_cv_`
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +53,99 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the runtime module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The runtime module interface type.*/
+enum InterfaceType {
+ INPUT = 0,
+ OUTPUT,
+};
+/*!
+ *\brief The structure includes the module index and the module output index.
+ */
+struct ModuleInterfaceID {
+ ModuleInterfaceID() : runtime_idx(0), runtime_interface_idx(0), interface_type(OUTPUT) { ; }
+ ModuleInterfaceID(int runtime_index, int runtime_interface_index, InterfaceType type = OUTPUT) {
+ runtime_idx = runtime_index;
+ runtime_interface_idx = runtime_interface_index;
+ interface_type = type;
+ }
+ int runtime_idx;
+ union {
+ /*!\brief The output interface index.*/
+ int runtime_output_idx;
+ /*!\brief The input interface index.*/
+ int runtime_input_idx;
+ /*!\brief The interface index.*/
+ int runtime_interface_idx;
+ };
+ /*!\brief The interface type*/
+ InterfaceType interface_type;
+};
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable notify_cv;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ std::atomic<bool> exit_state_{false};
+ /*!
+ * \brief The 'ModuleInterfaceID' in which the data was ready and triggered this
+ * notification.
+ */
+ ModuleInterfaceID notification_source_;
+
+ public:
+ /*!
+ * \brief Constructing the DataNotify class.
+ * \param parent_output_id The id of a runtime interface which is sending out the data
+ * notification.
+ */
+ explicit DataNotify(ModuleInterfaceID parent_output_id) {
+ notification_source_ = parent_output_id;
+ }
+ /*!
+ * \brief Getting the notification source.
+ * \return The first 'int' is the runtime index, and the second 'int' is the output index.
+ */
+ ModuleInterfaceID GetNotifySource(void) { return notification_source_; }
+ /*!
+ *\brief Waiting for the notification.
+ *\return Returning the value 'false' when the notification is in a 'exit' state, else
+ * return true.
+ */
+ bool Wait(void) {
+ std::unique_lock<std::mutex> lock(mutex_);
+ notify_cv.wait(lock, [&] { return this->data_ready_; });
+ data_ready_ = false;
+ return !exit_state_;
Review comment:
It should return `!GetExitState()`.
##########
File path: src/runtime/pipeline/pipeline_scheduler.cc
##########
@@ -101,18 +106,11 @@ void PipelineScheduler::PipelineRunSequential(
void PipelineScheduler::PipelineRun(const std::vector<std::shared_ptr<BackendRuntime>>& runtimes,
ConfigPipelineExecution pipeline_config, bool sequential_mode) {
if (!sequential_mode) {
- // TODO(huajsj) remove this check after all of pipeline features in.
- LOG(FATAL) << "Currently only supports sequential mode.";
+ runtimes.front()->RunPipeLine();
Review comment:
It is a little bit messy to use `PipeLine` and `Pipeline` interchangeably, I think.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r810787674
##########
File path: src/runtime/pipeline/pipeline_struct.h
##########
@@ -42,6 +52,75 @@ using GlobalOutputPair = std::pair<int, int>;
* The first 'int' is the module index, and the second 'int' is the module output index.
*/
using ModuleOutputPair = std::pair<int, int>;
+/*!
+ *\brief The pair includes the module index and the module input index.
+ * The first 'int' is the module index, and the second 'int' is the module input index.
+ */
+using ModuleInputPair = std::pair<int, int>;
+/*!\brief The data notification structure.*/
+class DataNotify {
+ private:
+ /*!\brief The 'contitional variable' is used to wait for notification.*/
+ std::condition_variable condition_;
+ /*!\brief The mutex is used to protect the 'conditional variable'.*/
+ std::mutex mutex_;
+ /*!\brief Whether a data is ready or not.*/
+ volatile bool data_ready_ = false;
+ /*!\brief Whether the thread should exit or not.*/
+ volatile bool exit_state_ = false;
Review comment:
> seems like not need to use atomic, because 'data_ready_' already protected by 'mutex_' and for exit_state, there is only a single 'writing' in ExitNotify then no race condition can happen
Then why do you need `volatile`? I'm not really familiar with `volatile` except that it is generally considered as a bad practice.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] masahi commented on a change in pull request #10234: [Runtime][Pipeline Executor] multiple threads management and the data forwarding notification mechanism.
Posted by GitBox <gi...@apache.org>.
masahi commented on a change in pull request #10234:
URL: https://github.com/apache/tvm/pull/10234#discussion_r810789466
##########
File path: src/runtime/pipeline/pipeline_executor.cc
##########
@@ -79,8 +79,10 @@ PackedFunc PipelineExecutor::GetFunction(const std::string& name,
[sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { *rv = this->GetOutput(); });
} else if (name == "run") {
return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { this->Run(args[0]); });
- } else if (name == "stop") {
- return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) { this->Stop(); });
+ } else if (name == "get_statistic_pipe_execute_number") {
+ return PackedFunc([sptr_to_self, this](TVMArgs args, TVMRetValue* rv) {
+ *rv = this->PipelineStatisticPipeExecuteNumber();
Review comment:
As I said, it's not clear what `Number` means. How about `PipelineStatisticPipeExecuterCount()`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org