Merge pull request #3245 from STEllAR-GROUP/apex_refactoring_with_guids

Apex refactoring with guids

CMakeLists.txt

@@ -1524,7 +1524,7 @@ if(HPX_WITH_APEX)
   include(GitExternal)
   git_external(apex
     https://github.com/khuck/xpress-apex.git
-    master
+    v2.0
     ${_hpx_apex_no_update}
     VERBOSE)
 

hpx/runtime/threads/coroutines/coroutine.hpp

@@ -97,10 +97,14 @@ namespace hpx { namespace threads { namespace coroutines
         }
 
 #if defined(HPX_HAVE_APEX)
-        void** get_apex_data() const
+        void* get_apex_data() const
         {
             return impl_.get_apex_data();
         }
+        void set_apex_data(void * data)
+        {
+            return impl_.set_apex_data(data);
+        }
 #endif
 
         void rebind(functor_type&& f, thread_id_type id)

hpx/runtime/threads/coroutines/detail/context_base.hpp

@@ -48,6 +48,9 @@
 #include <hpx/runtime/threads/coroutines/exception.hpp>
 #include <hpx/runtime/threads/thread_id_type.hpp>
 #include <hpx/util/assert.hpp>
+#if defined(HPX_HAVE_APEX)
+#include <hpx/util/apex.hpp>
+#endif
 
 #include <atomic>
 #include <cstddef>
@@ -250,15 +253,19 @@ namespace hpx { namespace threads { namespace coroutines { namespace detail
         }
 
 #if defined(HPX_HAVE_APEX)
-        void** get_apex_data() const
+        void* get_apex_data() const
         {
             // APEX wants the ADDRESS of a location to store
             // data.  This storage could be updated asynchronously,
             // so APEX stores this address, and uses it as a way
             // to remember state for the HPX thread in-betweeen
             // calls to apex::start/stop/yield/resume().
             // APEX will change the value pointed to by the address.
-            return const_cast<void**>(&m_apex_data);
+            return m_apex_data;
+        }
+        void set_apex_data(void * data)
+        {
+            m_apex_data = data;
         }
 #endif
 
@@ -338,7 +345,7 @@ namespace hpx { namespace threads { namespace coroutines { namespace detail
 #endif
             HPX_ASSERT(m_thread_data == 0);
 #if defined(HPX_HAVE_APEX)
-            HPX_ASSERT(m_apex_data == 0ull);
+            m_apex_data = 0ull;
 #endif
             m_type_info = std::exception_ptr();
         }

hpx/runtime/threads/coroutines/detail/coroutine_self.hpp

@@ -197,11 +197,16 @@ namespace hpx { namespace threads { namespace coroutines { namespace detail
         static HPX_EXPORT void reset_self();
 
 #if defined(HPX_HAVE_APEX)
-        void** get_apex_data() const
+        void* get_apex_data(void) const
         {
             HPX_ASSERT(m_pimpl);
             return m_pimpl->get_apex_data();
         }
+        void set_apex_data(void * data)
+        {
+            HPX_ASSERT(m_pimpl);
+            m_pimpl->set_apex_data(data);
+        }
 #endif
 
     private:

hpx/runtime/threads/detail/scheduling_loop.hpp

@@ -355,30 +355,7 @@ namespace hpx { namespace threads { namespace detail
                     if (HPX_LIKELY(thrd_stat.is_valid() &&
                             thrd_stat.get_previous() == pending))
                     {
-#if defined(HPX_HAVE_APEX)
-                        // get the APEX data pointer, in case we are resuming the
-                        // thread and have to restore any leaf timers from
-                        // direct actions, etc.
-
-                        // the address of tmp_data is getting stored
-                        // by APEX during this call
-                        util::apex_wrapper apex_profiler(
-                            background_thread->get_description(),
-                            background_thread->get_apex_data());
-
-                        thrd_stat = (*background_thread)();
-
-                        if (thrd_stat.get_previous() == terminated)
-                        {
-                            apex_profiler.stop();
-                        }
-                        else
-                        {
-                            apex_profiler.yield();
-                        }
-#else
                         thrd_stat = (*background_thread)();
-#endif
                         thread_data* next = thrd_stat.get_next_thread();
                         if (next != nullptr && next != background_thread.get())
                         {
@@ -527,14 +504,15 @@ namespace hpx { namespace threads { namespace detail
                                 // the address of tmp_data is getting stored
                                 // by APEX during this call
                                 util::apex_wrapper apex_profiler(
-                                    thrd->get_description(),
                                     thrd->get_apex_data());
 
                                 thrd_stat = (*thrd)();
 
                                 if (thrd_stat.get_previous() == terminated)
                                 {
                                     apex_profiler.stop();
+                                    // just in case, clean up the now dead pointer.
+                                    thrd->set_apex_data(nullptr);
                                 }
                                 else
                                 {

hpx/runtime/threads/thread_data.hpp

@@ -526,10 +526,14 @@ namespace hpx { namespace threads
         }
 
 #if defined(HPX_HAVE_APEX)
-        void** get_apex_data() const
+        void* get_apex_data() const
         {
             return coroutine_.get_apex_data();
         }
+        void set_apex_data(void * data)
+        {
+            return coroutine_.set_apex_data(data);
+        }
 #endif
 
         void rebind(thread_init_data& init_data,
@@ -598,6 +602,9 @@ namespace hpx { namespace threads
             }
             if (0 == parent_locality_id_)
                 parent_locality_id_ = get_locality_id();
+#endif
+#if defined(HPX_HAVE_APEX)
+            set_apex_data(apex_new_task(get_description()));
 #endif
             HPX_ASSERT(init_data.stacksize != 0);
             HPX_ASSERT(coroutine_.is_ready());

hpx/runtime/threads/thread_data_fwd.hpp

@@ -159,7 +159,8 @@ namespace hpx { namespace threads
         thread_state_enum state = unknown);
 
 #if defined(HPX_HAVE_APEX)
-    HPX_API_EXPORT void** get_self_apex_data();
+    HPX_API_EXPORT void* get_self_apex_data(void);
+    HPX_API_EXPORT void set_self_apex_data(void * data);
 #endif
 }}
 

hpx/util/annotated_function.hpp

@@ -73,16 +73,18 @@ namespace hpx { namespace util
         HPX_NON_COPYABLE(annotate_function);
 
         explicit annotate_function(char const* name)
-          : apex_profiler_(name, threads::get_self_apex_data())
-        {}
+        {
+            threads::set_self_apex_data(
+                apex_update_task(threads::get_self_apex_data(),
+                name));
+        }
         template <typename F>
         explicit annotate_function(F && f)
-          : apex_profiler_(hpx::util::thread_description(f),
-                threads::get_self_apex_data())
-        {}
-
-    private:
-        hpx::util::apex_wrapper apex_profiler_;
+        {
+            threads::set_self_apex_data(
+                apex_update_task(threads::get_self_apex_data(),
+                hpx::util::thread_description(f)));
+        }
     };
 #else
     struct annotate_function

hpx/util/apex.hpp

@@ -35,36 +35,49 @@ namespace hpx { namespace util
         apex::finalize();
     }
 
-    struct apex_wrapper
+    inline void * apex_new_task(
+                thread_description const& description)
     {
-        apex_wrapper(thread_description const& name)
-          : name_(name), stopped(false)
-        {
-            if (name_.kind() == thread_description::data_type_description)
-            {
-                profiler_ = apex::start(name_.get_description());
-            }
-            else
-            {
-                profiler_ = apex::start(
-                    apex_function_address(name_.get_address()));
-            }
+        if (description.kind() ==
+                thread_description::data_type_description) {
+            return (void*)apex::new_task(description.get_description());
+        } else {
+            return (void*)apex::new_task(description.get_address());
         }
-        apex_wrapper(thread_description const& name, void** const data_ptr)
-          : name_(name), stopped(false)
+    }
+
+    inline void * apex_new_task(char const* name)
+    {
+        return (void*)apex::new_task(name);
+    }
+
+    inline void * apex_update_task(void * wrapper,
+                thread_description const& description)
+    {
+        if (description.kind() == thread_description::data_type_description) {
+            return (void*)apex::update_task((apex::task_wrapper*)wrapper,
+                description.get_description());
+        } else {
+            return (void*)apex::update_task((apex::task_wrapper*)wrapper,
+                description.get_address());
+        }
+    }
+
+    inline void * apex_update_task(void * wrapper, char const* name)
+    {
+        return (void*)apex::update_task((apex::task_wrapper*)wrapper, name);
+    }
+
+    struct apex_wrapper
+    {
+        apex_wrapper(void* const data_ptr) : stopped(false), data_(nullptr)
         {
-            if (name_.kind() == thread_description::data_type_description)
-            {
-                // not a mistake... we need to pass in the address of this
-                // pointer, so that we can assign data to it in apex...
-                profiler_ = apex::start(name_.get_description(), data_ptr);
-            }
-            else
-            {
-                // not a mistake... we need to pass in the address of this
-                // pointer, so that we can assign data to it in apex...
-                profiler_ = apex::start(
-                    apex_function_address(name_.get_address()), data_ptr);
+            /* APEX internal actions are not timed.  Otherwise, we would
+             * end up with recursive timers. So it's possible to have
+             * a null task wrapper pointer here. */
+            if (data_ptr != nullptr) {
+                data_ = (apex::task_wrapper*)data_ptr;
+                apex::start(data_);
             }
         }
         ~apex_wrapper()
@@ -75,20 +88,29 @@ namespace hpx { namespace util
         void stop() {
             if(!stopped) {
                 stopped = true;
-                apex::stop(profiler_);
+            /* APEX internal actions are not timed.  Otherwise, we would
+             * end up with recursive timers. So it's possible to have
+             * a null task wrapper pointer here. */
+                if (data_ != nullptr) {
+                    apex::stop(data_);
+                }
             }
         }
 
         void yield() {
             if(!stopped) {
                 stopped = true;
-                apex::yield(profiler_);
+            /* APEX internal actions are not timed.  Otherwise, we would
+             * end up with recursive timers. So it's possible to have
+             * a null task wrapper pointer here. */
+                if (data_ != nullptr) {
+                    apex::yield(data_);
+                }
             }
         }
 
-        thread_description name_;
         bool stopped;
-        apex::profiler * profiler_;
+        apex::task_wrapper * data_;
     };
 
     struct apex_wrapper_init

src/runtime/threads/thread_data.cpp

@@ -214,12 +214,19 @@ namespace hpx { namespace threads
     }
 
 #if defined(HPX_HAVE_APEX)
-    void** get_self_apex_data()
+    void* get_self_apex_data()
     {
         thread_self* self = get_self_ptr();
         if (nullptr == self)
             return nullptr;
         return self->get_apex_data();
     }
+    void set_self_apex_data(void* data)
+    {
+        thread_self* self = get_self_ptr();
+        if (nullptr == self)
+            return;
+        self->set_apex_data(data);
+    }
 #endif
 }}