From b2163b24e29f0afa39472d3fd6265cebd8cafbff Mon Sep 17 00:00:00 2001 From: "Samuel K. Gutierrez" Date: Mon, 12 Aug 2024 12:44:06 -0600 Subject: [PATCH] Cleanup thread split interfaces. Signed-off-by: Samuel K. Gutierrez --- src/quo-vadis-pthread.cc | 2 +- src/qvi-bbuff-rmi.h | 6 +-- src/qvi-hwsplit.cc | 100 ++++++++++++++++++++++++++------------- src/qvi-hwsplit.h | 51 ++++++++------------ src/qvi-scope.cc | 46 ++++-------------- src/qvi-scope.h | 2 +- 6 files changed, 99 insertions(+), 108 deletions(-) diff --git a/src/quo-vadis-pthread.cc b/src/quo-vadis-pthread.cc index ad65e09..750a5c3 100644 --- a/src/quo-vadis-pthread.cc +++ b/src/quo-vadis-pthread.cc @@ -67,7 +67,7 @@ qv_pthread_scope_split( return QV_ERR_INVLD_ARG; } try { - return scope->thsplit( + return scope->thread_split( npieces, color_array, nthreads, QV_HW_OBJ_LAST, subscope ); } diff --git a/src/qvi-bbuff-rmi.h b/src/qvi-bbuff-rmi.h index 474ae58..3b6c723 100644 --- a/src/qvi-bbuff-rmi.h +++ b/src/qvi-bbuff-rmi.h @@ -16,7 +16,7 @@ * a = size_t * b = qvi_bbuff_rmi_bytes_in_t, qvi_bbuff_rmi_bytes_out_t * c = hwloc_cpuset_t - * c = qvi_hwloc_bitmap_s + * c = qvi_hwloc_bitmap * d = qv_scope_create_hints_t * h = qvi_hwpool * * i = int @@ -479,7 +479,7 @@ qvi_bbuff_rmi_pack_item( } /** - * Packs qvi_hwloc_bitmap_s + * Packs qvi_hwloc_bitmap */ inline int qvi_bbuff_rmi_pack_item( @@ -777,7 +777,7 @@ qvi_bbuff_rmi_unpack_item( } /** - * Unpacks qvi_hwloc_bitmap_s. + * Unpacks qvi_hwloc_bitmap. */ inline int qvi_bbuff_rmi_unpack_item( diff --git a/src/qvi-hwsplit.cc b/src/qvi-hwsplit.cc index 6202573..052376e 100644 --- a/src/qvi-hwsplit.cc +++ b/src/qvi-hwsplit.cc @@ -170,30 +170,6 @@ qvi_hwsplit::cpuset(void) const return m_hwpools[0]->cpuset(); } -std::vector & -qvi_hwsplit::tids(void) -{ - return m_group_tids; -} - -std::vector & -qvi_hwsplit::colors(void) -{ - return m_colors; -} - -std::vector & -qvi_hwsplit::hwpools(void) -{ - return m_hwpools; -} - -qvi_hwloc_cpusets_t & -qvi_hwsplit::affinities(void) -{ - return m_affinities; -} - int qvi_hwsplit::split_cpuset( qvi_hwloc_cpusets_t &result @@ -218,6 +194,63 @@ qvi_hwsplit::split_cpuset( return rc; } +int +qvi_hwsplit::thread_split( + qv_scope_t *parent, + uint_t npieces, + int *kcolors, + uint_t k, + qv_hw_obj_type_t maybe_obj_type, + qvi_hwpool ***khwpools +) { + const uint_t group_size = k; + // Construct the hardware split. + qvi_hwsplit hwsplit(parent, group_size, npieces, maybe_obj_type); + // Eagerly make room for the group member information. + hwsplit.reserve(); + // Since this is called by a single task, get its ID and associated + // hardware affinity here, and replicate them in the following loop + // that populates splitagg. + //No point in doing this in a loop. + const pid_t taskid = qvi_task::mytid(); + // Get the task's current affinity. + hwloc_cpuset_t task_affinity = nullptr; + int rc = parent->group()->task()->bind_top(&task_affinity); + if (qvi_unlikely(rc != QV_SUCCESS)) return rc; + // Prepare the hwsplit with our parent's information. + for (uint_t i = 0; i < group_size; ++i) { + // Store requested colors in aggregate. + hwsplit.m_colors[i] = kcolors[i]; + // Since the parent hardware pool is the resource we are splitting and + // agg_split_* calls expect |group_size| elements, replicate by dups. + rc = qvi_dup(*parent->hwpool(), &hwsplit.m_hwpools[i]); + if (qvi_unlikely(rc != QV_SUCCESS)) break; + // Since this is called by a single task, replicate its task ID, too. + hwsplit.m_group_tids[i] = taskid; + // Same goes for the task's affinity. + hwsplit.m_affinities[i].set(task_affinity); + } + // Cleanup: we don't need task_affinity anymore. + qvi_hwloc_bitmap_delete(&task_affinity); + if (qvi_unlikely(rc != QV_SUCCESS)) return rc; + // Split the hardware resources based on the provided split parameters. + rc = hwsplit.split(); + if (qvi_unlikely(rc != QV_SUCCESS)) return rc; + // Now populate the hardware pools as the result. + qvi_hwpool **ikhwpools = new qvi_hwpool *[group_size]; + for (uint_t i = 0; i < group_size; ++i) { + // Copy out, since the hardware pools in splitagg will get freed. + rc = qvi_dup(*hwsplit.m_hwpools[i], &ikhwpools[i]); + if (qvi_unlikely(rc != QV_SUCCESS)) break; + } + if (qvi_unlikely(rc != QV_SUCCESS)) { + delete[] ikhwpools; + ikhwpools = nullptr; + } + *khwpools = ikhwpools; + return rc; +} + int qvi_hwsplit::osdev_cpusets( qvi_hwloc_cpusets_t &result @@ -272,7 +305,6 @@ qvi_hwsplit::affinity_preserving_policy(void) const } } -/** Releases all devices contained in the provided split aggregate. */ int qvi_hwsplit::release_devices(void) { @@ -452,12 +484,12 @@ int qvi_hwsplit::split_affinity_preserving_pass1(void) { // cpusets used for first mapping pass. - qvi_hwloc_cpusets_t cpusets{}; + qvi_hwloc_cpusets_t cpusets; // Get the primary cpusets used for the first pass of mapping. int rc = primary_cpusets(cpusets); if (rc != QV_SUCCESS) return rc; // Maintains the mapping between task (consumer) IDs and resource IDs. - qvi_map_t map{}; + qvi_map_t map; // Map tasks based on their affinity to resources encoded by the cpusets. const auto policy = affinity_preserving_policy(); rc = qvi_map_affinity_preserving( @@ -735,24 +767,24 @@ qvi_hwsplit_coll::gather_hwpools( int qvi_hwsplit_coll::gather(void) { - int rc = gather_values(qvi_task::mytid(), m_hwsplit.tids()); + int rc = gather_values(qvi_task::mytid(), m_hwsplit.m_group_tids); if (qvi_unlikely(rc != QV_SUCCESS)) return rc; - rc = gather_values(m_color, m_hwsplit.colors()); + rc = gather_values(m_color, m_hwsplit.m_colors); if (qvi_unlikely(rc != QV_SUCCESS)) return rc; // Note that the result hwpools are copies, so we can modify them freely. - rc = gather_hwpools(m_parent->hwpool(), m_hwsplit.hwpools()); + rc = gather_hwpools(m_parent->hwpool(), m_hwsplit.m_hwpools); if (qvi_unlikely(rc != QV_SUCCESS)) return rc; const int myrank = m_parent->group()->rank(); const uint_t group_size = m_parent->group()->size(); if (myrank == qvi_hwsplit_coll::rootid) { - m_hwsplit.affinities().resize(group_size); + m_hwsplit.m_affinities.resize(group_size); for (uint_t tid = 0; tid < group_size; ++tid) { hwloc_cpuset_t cpuset = nullptr; rc = m_parent->group()->task()->bind_top(&cpuset); if (qvi_unlikely(rc != QV_SUCCESS)) break; // - rc = m_hwsplit.affinities()[tid].set(cpuset); + rc = m_hwsplit.m_affinities[tid].set(cpuset); // Clean up. qvi_hwloc_bitmap_delete(&cpuset); if (qvi_unlikely(rc != QV_SUCCESS)) break; @@ -806,9 +838,9 @@ qvi_hwsplit_coll::scatter( int *colorp, qvi_hwpool **result ) { - const int rc = scatter_values(m_hwsplit.colors(), colorp); + const int rc = scatter_values(m_hwsplit.m_colors, colorp); if (qvi_unlikely(rc != QV_SUCCESS)) return rc; - return scatter_hwpools(m_hwsplit.hwpools(), result); + return scatter_hwpools(m_hwsplit.m_hwpools, result); } int diff --git a/src/qvi-hwsplit.h b/src/qvi-hwsplit.h index c24371f..9dc7f14 100644 --- a/src/qvi-hwsplit.h +++ b/src/qvi-hwsplit.h @@ -19,6 +19,8 @@ #include "qvi-hwpool.h" #include "qvi-map.h" +struct qvi_hwsplit_coll; + /** * Hardware split aggregation: a collection of information relevant to split * operations requiring aggregated (e.g., global) knowledge to perform a split. @@ -30,6 +32,7 @@ * this structure, but that isn't a requirement. */ struct qvi_hwsplit { + friend qvi_hwsplit_coll; private: /** A pointer to my RMI. */ qvi_rmi_client_t *m_rmi = nullptr; @@ -92,18 +95,6 @@ struct qvi_hwsplit { */ qvi_hwloc_bitmap cpuset(void) const; - /** Returns a reference to the group TIDs. */ - std::vector & - tids(void); - /** Returns a reference to the group colors. */ - std::vector & - colors(void); - /** Returns a reference to the group hardware pools. */ - std::vector & - hwpools(void); - /** Returns a reference to the group task affinities. */ - qvi_hwloc_cpusets_t & - affinities(void); /** * Performs a straightforward splitting of the provided cpuset: * split the provided base cpuset into split_size distinct pieces. @@ -112,9 +103,17 @@ struct qvi_hwsplit { split_cpuset( qvi_hwloc_cpusets_t &result ) const; - /** - * Returns device affinities that are part of the split. - */ + /** Performs a thread-split operation, returns relevant hardare pools. */ + static int + thread_split( + qv_scope_t *parent, + uint_t npieces, + int *kcolors, + uint_t k, + qv_hw_obj_type_t maybe_obj_type, + qvi_hwpool ***khwpools + ); + /** Returns device affinities that are part of the split. */ int osdev_cpusets( qvi_hwloc_cpusets_t &result @@ -127,35 +126,25 @@ struct qvi_hwsplit { qvi_map_fn_t affinity_preserving_policy(void) const; - /** Releases all devices contained in the provided split aggregate. */ + /** Releases all devices contained in the hardware split. */ int release_devices(void); - /** - * Straightforward user-defined device splitting. - */ + /** Straightforward user-defined device splitting. */ int split_devices_user_defined(void); - /** - * Affinity preserving device splitting. - */ + /** Affinity preserving device splitting. */ int split_devices_affinity_preserving(void); - /** - * User-defined split. - */ + /** User-defined split. */ int split_user_defined(void); int split_affinity_preserving_pass1(void); - /** - * Affinity preserving split. - */ + /** Affinity preserving split. */ int split_affinity_preserving(void); - /** - * Splits aggregate scope data. - */ + /** Splits aggregate scope data. */ int split(void); }; diff --git a/src/qvi-scope.cc b/src/qvi-scope.cc index 1fda238..c346232 100644 --- a/src/qvi-scope.cc +++ b/src/qvi-scope.cc @@ -264,7 +264,7 @@ qv_scope::split_at( } int -qv_scope::thsplit( +qv_scope::thread_split( uint_t npieces, int *kcolors, uint_t k, @@ -272,38 +272,12 @@ qv_scope::thsplit( qv_scope_t ***thchildren ) { *thchildren = nullptr; - const uint_t group_size = k; - // Construct the hardware split. - qvi_hwsplit hwsplit(this, group_size, npieces, maybe_obj_type); - // Eagerly make room for the group member information. - hwsplit.reserve(); - // Since this is called by a single task, get its ID and associated - // hardware affinity here, and replicate them in the following loop - // that populates splitagg. - //No point in doing this in a loop. - const pid_t taskid = qvi_task::mytid(); - hwloc_cpuset_t task_affinity = nullptr; - // Get the task's current affinity. - int rc = m_group->task()->bind_top(&task_affinity); - if (rc != QV_SUCCESS) return rc; - for (uint_t i = 0; i < group_size; ++i) { - // Store requested colors in aggregate. - hwsplit.colors()[i] = kcolors[i]; - // Since the parent hardware pool is the resource we are splitting and - // agg_split_* calls expect |group_size| elements, replicate by dups. - rc = qvi_dup(*m_hwpool, &hwsplit.hwpools()[i]); - if (rc != QV_SUCCESS) break; - // Since this is called by a single task, replicate its task ID, too. - hwsplit.tids()[i] = taskid; - // Same goes for the task's affinity. - hwsplit.affinities()[i].set(task_affinity); - } - // Cleanup: we don't need task_affinity anymore. - qvi_hwloc_bitmap_delete(&task_affinity); - if (rc != QV_SUCCESS) return rc; - // Split the hardware resources based on the provided split parameters. - rc = hwsplit.split(); + // Split the hardware, get the hardare pools. + qvi_hwpool **hwpools = nullptr; + int rc = qvi_hwsplit::thread_split( + this, npieces, kcolors, k, maybe_obj_type, &hwpools + ); if (rc != QV_SUCCESS) return rc; // Split off from our parent group. This call is called from a context in // which a process is splitting its resources across threads, so create a @@ -314,13 +288,9 @@ qv_scope::thsplit( // Now create and populate the children. qv_scope_t **ithchildren = new qv_scope_t *[group_size]; for (uint_t i = 0; i < group_size; ++i) { - // Copy out, since the hardware pools in splitagg will get freed. - qvi_hwpool *hwpool = nullptr; - rc = qvi_dup(*hwsplit.hwpools()[i], &hwpool); - if (rc != QV_SUCCESS) break; // Create and initialize the new scope. qv_scope_t *child = nullptr; - rc = qvi_new(&child, thgroup, hwpool); + rc = qvi_new(&child, thgroup, hwpools[i]); if (rc != QV_SUCCESS) break; thgroup->retain(); ithchildren[i] = child; @@ -344,7 +314,7 @@ qv_scope::thsplit_at( uint_t k, qv_scope_t ***kchildren ) { - return thsplit(hwpool_nobjects(type), kgroup_ids, k, type, kchildren); + return thread_split(hwpool_nobjects(type), kgroup_ids, k, type, kchildren); } /* diff --git a/src/qvi-scope.h b/src/qvi-scope.h index 986b066..a46a930 100644 --- a/src/qvi-scope.h +++ b/src/qvi-scope.h @@ -111,7 +111,7 @@ struct qv_scope { ); int - thsplit( + thread_split( uint_t npieces, int *kcolors, uint_t k,