From 80bb555ddec2b0aff6eceb9e39f9a3e3fcfd28d3 Mon Sep 17 00:00:00 2001 From: Christian Mauch <cmauch@kip.uni-heidelberg.de> Date: Mon, 6 Mar 2023 14:26:36 +0100 Subject: [PATCH] Rework quiggeldy enforce functionality Depends-On:19662,19686 Change-Id: I8c7df05b423a8b32b1dc38ef2ba0e3835793287d --- .../cerealization_quiggeldy_interface_types.h | 1 + include/hxcomm/common/quiggeldy_connection.tcc | 3 +++ .../hxcomm/common/quiggeldy_interface_types.h | 5 ++++- include/hxcomm/common/quiggeldy_worker.h | 5 ++++- include/hxcomm/common/quiggeldy_worker.tcc | 9 ++++++--- include/hxcomm/common/reinit_stack.h | 5 +++++ include/hxcomm/common/reinit_stack.tcc | 9 +++++++++ include/hxcomm/common/reinit_stack_entry.h | 8 +++----- include/hxcomm/common/reinit_stack_entry.tcc | 18 ++++++------------ .../include/pyhxcomm/vx/reinit_stack_entry.h | 5 ++--- tests/sw/hxcomm/test-quiggeldy.cpp | 2 +- 11 files changed, 44 insertions(+), 26 deletions(-) diff --git a/include/hxcomm/common/cerealization_quiggeldy_interface_types.h b/include/hxcomm/common/cerealization_quiggeldy_interface_types.h index 5ecba37..986a64d 100644 --- a/include/hxcomm/common/cerealization_quiggeldy_interface_types.h +++ b/include/hxcomm/common/cerealization_quiggeldy_interface_types.h @@ -11,6 +11,7 @@ void CEREAL_SERIALIZE_FUNCTION_NAME( { ar(CEREAL_NVP(reinit_entry.request)); ar(CEREAL_NVP(reinit_entry.snapshot)); + ar(CEREAL_NVP(reinit_entry.reinit_pending)); } } // namespace cereal diff --git a/include/hxcomm/common/quiggeldy_connection.tcc b/include/hxcomm/common/quiggeldy_connection.tcc index 2b0b125..1713221 100644 --- a/include/hxcomm/common/quiggeldy_connection.tcc +++ b/include/hxcomm/common/quiggeldy_connection.tcc @@ -345,6 +345,9 @@ QuiggeldyConnection<ConnectionParameter, RcfClient>::submit_blocking( typename interface_types::response_type response = client->submit_work(request, sequence_num); accumulate_time_info(response.second); + + // quiggeldy server done with execution, current reinit stack can be set to done + this->m_reinit_stack->set_all_done(); return response; }); } diff --git a/include/hxcomm/common/quiggeldy_interface_types.h b/include/hxcomm/common/quiggeldy_interface_types.h index 311bc65..57c38d4 100644 --- a/include/hxcomm/common/quiggeldy_interface_types.h +++ b/include/hxcomm/common/quiggeldy_interface_types.h @@ -18,8 +18,11 @@ struct ReinitEntryType using request_type = detail::execute_messages_argument_t<ConnectionParameter>; request_type request; - std::optional<request_type> snapshot; + // State flag if reinit needs to be executed. Set to false if reinit entry should only be + // executed on session switch. + bool reinit_pending = true; + typedef typename ConnectionParameter::QuiggeldyScheduleOutToInTransform transform_type; }; diff --git a/include/hxcomm/common/quiggeldy_worker.h b/include/hxcomm/common/quiggeldy_worker.h index 582df2f..9a3a854 100644 --- a/include/hxcomm/common/quiggeldy_worker.h +++ b/include/hxcomm/common/quiggeldy_worker.h @@ -89,8 +89,11 @@ public: * This function is called whenever we had to relinquish control of our * hardware resource and the user specified a reinit-program to be loaded * prior to the next work-unit being executed. + * + * @param reinit_type reinit stack to be executed + * @param enforce execute all reinit stack entries */ - void perform_reinit(reinit_type const&); + void perform_reinit(reinit_type& reinit_type, bool enforce); /** * This function is called whenever we have to relinquish control of our diff --git a/include/hxcomm/common/quiggeldy_worker.tcc b/include/hxcomm/common/quiggeldy_worker.tcc index 4337a8e..fba7423 100644 --- a/include/hxcomm/common/quiggeldy_worker.tcc +++ b/include/hxcomm/common/quiggeldy_worker.tcc @@ -255,7 +255,7 @@ typename QuiggeldyWorker<Connection>::response_type QuiggeldyWorker<Connection>: } template <typename Connection> -void QuiggeldyWorker<Connection>::perform_reinit(reinit_type const& reinit) +void QuiggeldyWorker<Connection>::perform_reinit(reinit_type& reinit, bool force) { if (m_mock_mode) { HXCOMM_LOG_DEBUG(m_logger, "Running mock-reinit!"); @@ -265,8 +265,11 @@ void QuiggeldyWorker<Connection>::perform_reinit(reinit_type const& reinit) HXCOMM_LOG_TRACE(m_logger, "Performing reinit!"); try { - for (auto const& entry : reinit) { - execute_messages(*m_connection, entry.request); + for (auto& entry : reinit) { + if (entry.reinit_pending || force) { + execute_messages(*m_connection, entry.request); + entry.reinit_pending = false; + } } } catch (const std::exception& e) { // TODO: Implement proper exception handling diff --git a/include/hxcomm/common/reinit_stack.h b/include/hxcomm/common/reinit_stack.h index 98d110a..f2f7158 100644 --- a/include/hxcomm/common/reinit_stack.h +++ b/include/hxcomm/common/reinit_stack.h @@ -78,6 +78,11 @@ public: template <typename UploaderT> void upload(UploaderT&) const; + /** + * Set all reinit stack entries as done + */ + void set_all_done(); + private: log4cxx::LoggerPtr m_logger; diff --git a/include/hxcomm/common/reinit_stack.tcc b/include/hxcomm/common/reinit_stack.tcc index fc5b5c6..7be0b96 100644 --- a/include/hxcomm/common/reinit_stack.tcc +++ b/include/hxcomm/common/reinit_stack.tcc @@ -85,4 +85,13 @@ void ReinitStack<CP>::upload(UploaderT& uploader) const uploader.upload(cref); } +template <typename CP> +void ReinitStack<CP>::set_all_done() +{ + std::lock_guard lk{m_mutex}; + for (auto& entry : m_stack) { + entry.reinit_pending = false; + } +} + } // namespace hxcomm diff --git a/include/hxcomm/common/reinit_stack_entry.h b/include/hxcomm/common/reinit_stack_entry.h index d32edc5..de2dabc 100644 --- a/include/hxcomm/common/reinit_stack_entry.h +++ b/include/hxcomm/common/reinit_stack_entry.h @@ -67,9 +67,8 @@ public: * Takes ownership of the program. * * @param entry What to upload. - * @param enforce Whether to force usage of reinit. */ - void set(reinit_entry_type&& entry, bool enforce = true); + void set(reinit_entry_type&& entry); /** * Register a reinit program to be used on the remote site. @@ -77,9 +76,8 @@ public: * Copies the program. * * @param entry What to upload. - * @param enforce Whether to force usage of reinit. */ - void set(reinit_entry_type const& entry, bool enforce = true); + void set(reinit_entry_type const& entry); /** * Enforce reinit program to be used on the remote site. @@ -108,7 +106,7 @@ private: template <typename Connection> void setup(Connection& connection); - void handle_unsupported_connection(reinit_entry_type const&, bool); + void handle_unsupported_connection(reinit_entry_type const&); }; } // namespace hxcomm diff --git a/include/hxcomm/common/reinit_stack_entry.tcc b/include/hxcomm/common/reinit_stack_entry.tcc index 8cf74e0..ce06eb1 100644 --- a/include/hxcomm/common/reinit_stack_entry.tcc +++ b/include/hxcomm/common/reinit_stack_entry.tcc @@ -63,10 +63,10 @@ void ReinitStackEntry<QuiggeldyConnection, ConnectionVariant>::pop() template <typename QuiggeldyConnection, typename ConnectionVariant> void ReinitStackEntry<QuiggeldyConnection, ConnectionVariant>::set( - reinit_entry_type&& entry, bool enforce_reinit) + reinit_entry_type&& entry) { if (!m_connection_supports_reinit) { - handle_unsupported_connection(entry, enforce_reinit); + handle_unsupported_connection(entry); } else if (!m_idx_in_stack) { throw std::runtime_error("Trying to update an already popped reinit stack entry value."); } else if (auto uploader = m_reinit_uploader.lock()) { @@ -79,9 +79,6 @@ void ReinitStackEntry<QuiggeldyConnection, ConnectionVariant>::set( #else std::ignore = entry; #endif - if (enforce_reinit) { - enforce(); - } } else { // Runtime check because ReinitStackEntry is exposed via Python bindings // and has to be valid for all connections. @@ -92,12 +89,12 @@ void ReinitStackEntry<QuiggeldyConnection, ConnectionVariant>::set( template <typename QuiggeldyConnection, typename ConnectionVariant> void ReinitStackEntry<QuiggeldyConnection, ConnectionVariant>::set( - reinit_entry_type const& entry, bool enforce_reinit) + reinit_entry_type const& entry) { // Runtime check because ReinitStackEntry is exposed via Python bindings // and has to be valid for all connections. if (!m_connection_supports_reinit) { - handle_unsupported_connection(entry, enforce_reinit); + handle_unsupported_connection(entry); } else if (auto uploader = m_reinit_uploader.lock()) { auto stack = m_reinit_stack.lock(); if (!stack->update_at(*m_idx_in_stack, entry)) { @@ -108,9 +105,6 @@ void ReinitStackEntry<QuiggeldyConnection, ConnectionVariant>::set( #else std::ignore = entry; #endif - if (enforce_reinit) { - enforce(); - } } else { HXCOMM_LOG_ERROR(m_logger, "Cannot register new reinit program: Uploader deleted."); throw std::runtime_error("Cannot register new reinit program: Uploader deleted."); @@ -143,9 +137,9 @@ void ReinitStackEntry<QuiggeldyConnection, ConnectionVariant>::setup(Connection& template <typename QuiggeldyConnection, typename ConnectionVariant> void ReinitStackEntry<QuiggeldyConnection, ConnectionVariant>::handle_unsupported_connection( - reinit_entry_type const& entry, bool enforce) + reinit_entry_type const& entry) { - if (enforce) { + if (entry.reinit_pending) { HXCOMM_LOG_TRACE( m_logger, "Connection does not support upload of reinit program, treating enforced " "reinit-like regular program to execute."); diff --git a/pyhxcomm/include/pyhxcomm/vx/reinit_stack_entry.h b/pyhxcomm/include/pyhxcomm/vx/reinit_stack_entry.h index 30ad8fd..f32e49e 100644 --- a/pyhxcomm/include/pyhxcomm/vx/reinit_stack_entry.h +++ b/pyhxcomm/include/pyhxcomm/vx/reinit_stack_entry.h @@ -55,11 +55,10 @@ GENPYBIND_MANUAL({ using reinit_stack_entry_t = typename wrapped_t::type; using reinit_entry_type = typename reinit_stack_entry_t::reinit_entry_type; - void (reinit_stack_entry_t::*call_ref)(reinit_entry_type const&, bool) = - &reinit_stack_entry_t::set; + void (reinit_stack_entry_t::*call_ref)(reinit_entry_type const&) = &reinit_stack_entry_t::set; wrapped.def("pop", &::hxcomm::vx::ReinitStackEntry::pop); - wrapped.def("set", call_ref, pybind11::arg("entry"), pybind11::arg("enforce") = true); + wrapped.def("set", call_ref, pybind11::arg("entry")); }) } // namespace pyhxcomm::vxGENPYBIND_TAG_HXCOMM_VX diff --git a/tests/sw/hxcomm/test-quiggeldy.cpp b/tests/sw/hxcomm/test-quiggeldy.cpp index 6dbf40f..7c45f42 100644 --- a/tests/sw/hxcomm/test-quiggeldy.cpp +++ b/tests/sw/hxcomm/test-quiggeldy.cpp @@ -115,7 +115,7 @@ TEST(Quiggeldy, SimpleMockModeReinit) StreamRC<decltype(client2)> stream2_rc{client2}; hxcomm::vx::ReinitStackEntry reinit{client1}; - reinit.set(typename decltype(client1)::interface_types::reinit_entry_type{}, false); + reinit.set(typename decltype(client1)::interface_types::reinit_entry_type{}); /* no reinit for stream 2 because no reinit program should also work */ // asynchronous calls -- GitLab