From 542f7d7b0b376586803ff72b6787e9d9e72a6c73 Mon Sep 17 00:00:00 2001 From: Thomas Heller Date: Wed, 11 Jul 2018 10:07:34 +0200 Subject: [PATCH] Fixing problems uncovered by amdados pilot - Data Item Manager: - First touch now follows the location of other existing requirements - Scheduling: - Work Items spawned as first, don't "split" the tree execution config --- allscale/data_item_manager/acquire.hpp | 36 +++++++----- allscale/data_item_manager/acquire_rank.hpp | 58 +++++++++++++++++--- allscale/data_item_manager/locate.hpp | 12 ++-- allscale/data_item_manager/location_info.hpp | 7 +++ allscale/data_item_reference.hpp | 4 +- allscale/data_item_requirement.hpp | 8 +++ allscale/detail/work_item_impl_base.hpp | 2 +- allscale/spawn.hpp | 4 ++ allscale/this_work_item.hpp | 4 +- allscale/work_item.hpp | 2 +- examples/CMakeLists.txt | 5 ++ src/components/scheduler_component.cpp | 3 + src/detail/work_item_impl_base.cpp | 4 +- src/this_work_item.cpp | 19 ++++++- tests/performance/CMakeLists.txt | 30 +++++----- tools/fixup_allscalecc.py | 2 + 16 files changed, 146 insertions(+), 54 deletions(-) diff --git a/allscale/data_item_manager/acquire.hpp b/allscale/data_item_manager/acquire.hpp index a4e3eac..855ee28 100644 --- a/allscale/data_item_manager/acquire.hpp +++ b/allscale/data_item_manager/acquire.hpp @@ -54,27 +54,35 @@ namespace allscale { namespace data_item_manager { HPX_ASSERT(!req.region.empty()); - // Resize data to the requested size... - auto& item = data_item_store::lookup(req.ref); - region_type req_region; - if (req.mode == access_mode::ReadOnly) - { - req_region = std::move(req.region); - } - else + region_type remainder = req.region; + for (auto& r: info.regions) { - boost::shared_lock l(item.region_mtx); - HPX_ASSERT(req.mode == access_mode::ReadWrite); - // clip region to registered region... - req_region = region_type::intersect(item.owned_region, req.region); + remainder = region_type::difference(remainder, r.second); } + + // Resize data to the requested size... + auto& item = data_item_store::lookup(req.ref); +// region_type req_region; +// if (req.mode == access_mode::ReadOnly) +// { +// req_region = std::move(req.region); +// } +// else +// { +// boost::shared_lock l(item.region_mtx); +// HPX_ASSERT(req.mode == access_mode::ReadWrite); +// // clip region to registered region... +// req_region = region_type::intersect(item.owned_region, req.region); +// } { std::unique_lock ll(item.fragment_mtx); + item.owned_region = + region_type::merge(item.owned_region, remainder); auto& frag = fragment(req.ref, item, ll); - if (!allscale::api::core::isSubRegion(req_region, frag.getCoveredRegion())) + if (!allscale::api::core::isSubRegion(req.region, frag.getCoveredRegion())) { frag.resize( - region_type::merge(req_region, frag.getCoveredRegion()) + region_type::merge(req.region, frag.getCoveredRegion()) ); } } diff --git a/allscale/data_item_manager/acquire_rank.hpp b/allscale/data_item_manager/acquire_rank.hpp index 66dcc84..a7f1128 100644 --- a/allscale/data_item_manager/acquire_rank.hpp +++ b/allscale/data_item_manager/acquire_rank.hpp @@ -34,14 +34,48 @@ namespace allscale { namespace data_item_manager { return std::size_t(-2); } - HPX_ASSERT(!info.regions.empty()); - // If we have more than one part, we need to split - if (info.regions.size() > 1) + using data_item_type = typename Requirement::data_item_type; + using lease_type = allscale::lease; + using transfer_action_type = transfer_action; + using region_type = typename data_item_type::region_type; + using mutex_type = typename data_item_store::data_item_type::mutex_type; + + // If we couldn't resolve a remainder, we need to allocate the requested + // fragment. This allocation happens either on this rank or the location + // of the other requirements. As such, we return -2. + if (info.regions.empty()) { - return std::size_t(-1); + return std::size_t(-2); } - return info.regions.begin()->first; + region_type remainder = req.region; + std::size_t dest_rank(-2); + for (auto& r: info.regions) + { + remainder = region_type::difference(remainder, r.second); + + if (r.first != dest_rank) + { + if (dest_rank != std::size_t(-2)) + { + return std::size_t(-1); + } + else + { + dest_rank = r.first; + } + } + } + + return dest_rank; +// HPX_ASSERT(!info.regions.empty()); +// // If we have more than one part, we need to split +// if (info.regions.size() > 1) +// { +// return std::size_t(-1); +// } +// +// return info.regions.begin()->first; } template void print_location_info(Requirement const& req, LocationInfo const& info) { - if (info.regions.size() > 1) +// if (info.regions.size() > 1) { - std::cerr << "Conflicting requirement " << typeid(req).name() << " with region: " << req.region << '\n'; + std::cerr << "Conflicting requirement " << typeid(req).name() << "(" << (req.mode == access_mode::ReadOnly? "ro" : "rw") << ", "<< std::hex << req.ref.id().get_lsb() << std::dec << ") with region: " << req.region << '\n'; std::cerr << "Cannot resolve locations on " << hpx::get_locality_id() << ":\n"; for (auto const& parts: info.regions) { diff --git a/allscale/data_item_manager/locate.hpp b/allscale/data_item_manager/locate.hpp index a1ea649..af4ac5e 100644 --- a/allscale/data_item_manager/locate.hpp +++ b/allscale/data_item_manager/locate.hpp @@ -263,12 +263,12 @@ namespace allscale { namespace data_item_manager { if (src_id == this_id && !remainder.empty()) { HPX_ASSERT(locate_state::init == state); - auto & part = info.regions[this_id]; - part = region_type::merge(part, remainder); - std::unique_lock l(item.region_mtx); - // merge with our own region - item.owned_region = - region_type::merge(item.owned_region, remainder); +// auto & part = info.regions[this_id]; +// part = region_type::merge(part, remainder); +// std::unique_lock l(item.region_mtx); +// // merge with our own region +// item.owned_region = +// region_type::merge(item.owned_region, remainder); #if defined(ALLSCALE_DEBUG_DIM) std::stringstream filename; filename << "data_item." << this_id << ".log"; diff --git a/allscale/data_item_manager/location_info.hpp b/allscale/data_item_manager/location_info.hpp index 83e371c..14b43db 100644 --- a/allscale/data_item_manager/location_info.hpp +++ b/allscale/data_item_manager/location_info.hpp @@ -12,6 +12,13 @@ namespace allscale { namespace data_item_manager { { location_info() = default; + location_info(location_info const& other) = default; + + location_info(location_info&& other) noexcept + : regions(std::move(other.regions)) + { + } + std::unordered_map regions; template diff --git a/allscale/data_item_reference.hpp b/allscale/data_item_reference.hpp index 284930b..8d535a1 100644 --- a/allscale/data_item_reference.hpp +++ b/allscale/data_item_reference.hpp @@ -70,7 +70,7 @@ namespace allscale { , cache(nullptr) {} - data_item_reference(data_item_reference&& other) + data_item_reference(data_item_reference&& other) noexcept : id_(std::move(other.id_)) , cache(nullptr) { @@ -85,7 +85,7 @@ namespace allscale { return *this; } - data_item_reference& operator=(data_item_reference&& other) + data_item_reference& operator=(data_item_reference&& other) noexcept { id_ = other.id_; cache.store(other.cache.load(std::memory_order_acquire), std::memory_order_release); diff --git a/allscale/data_item_requirement.hpp b/allscale/data_item_requirement.hpp index 557b69a..21cec91 100644 --- a/allscale/data_item_requirement.hpp +++ b/allscale/data_item_requirement.hpp @@ -21,6 +21,14 @@ namespace allscale{ data_item_requirement() = default; + data_item_requirement(data_item_requirement const&) = default; + data_item_requirement(data_item_requirement && other) noexcept + : ref(std::move(other.ref)) + , region(std::move(other.region)) + , mode(other.mode) + { + } + data_item_requirement( data_item_reference pref, typename DataItemType::region_type pregion, diff --git a/allscale/detail/work_item_impl_base.hpp b/allscale/detail/work_item_impl_base.hpp index f24bec4..73b41df 100644 --- a/allscale/detail/work_item_impl_base.hpp +++ b/allscale/detail/work_item_impl_base.hpp @@ -35,7 +35,7 @@ namespace allscale { namespace detail { id_.update_rank(rank); } - void set_this_id(machine_config const& config); + void set_this_id(machine_config const& config, bool is_first); this_work_item::id const& id() const; virtual const char* name() const=0; diff --git a/allscale/spawn.hpp b/allscale/spawn.hpp index a536675..d418991 100644 --- a/allscale/spawn.hpp +++ b/allscale/spawn.hpp @@ -51,6 +51,10 @@ namespace allscale typedef typename WorkItemDescription::result_type result_type; allscale::treeture tres(treeture_init); +// auto id = this_work_item::get_id_ptr(); +// if (id) +// id->reset_distribution(); + scheduler::schedule( work_item(true, WorkItemDescription(), deps.dep_, tres, std::forward(vs)...) ); diff --git a/allscale/this_work_item.hpp b/allscale/this_work_item.hpp index 0630a15..9ece733 100644 --- a/allscale/this_work_item.hpp +++ b/allscale/this_work_item.hpp @@ -43,12 +43,14 @@ namespace allscale { id(); - void set(detail::work_item_impl_base*, machine_config const& mconfig); + void set(detail::work_item_impl_base*, machine_config const& mconfig, bool); std::string name() const; std::size_t last() const; std::size_t depth() const; + void reset_distribution(); + std::size_t hash() const; id parent() const; diff --git a/allscale/work_item.hpp b/allscale/work_item.hpp index 942e64c..1874276 100644 --- a/allscale/work_item.hpp +++ b/allscale/work_item.hpp @@ -54,7 +54,7 @@ namespace allscale { void set_this_id(machine_config const& mconfig) { - impl_->set_this_id(mconfig); + impl_->set_this_id(mconfig, is_first_); } bool is_first() diff --git a/examples/CMakeLists.txt b/examples/CMakeLists.txt index 67a0403..02ad0c9 100644 --- a/examples/CMakeLists.txt +++ b/examples/CMakeLists.txt @@ -26,6 +26,11 @@ add_hpx_executable( SOURCES ipic3d_allscalecc_data.cpp COMPONENT_DEPENDENCIES allscale ) +add_hpx_executable( + amdados_allscalecc + SOURCES amdados/amdados_generated.cpp + COMPONENT_DEPENDENCIES allscale +) add_subdirectory(pfor) add_subdirectory(uts) diff --git a/src/components/scheduler_component.cpp b/src/components/scheduler_component.cpp index a402064..faf6190 100644 --- a/src/components/scheduler_component.cpp +++ b/src/components/scheduler_component.cpp @@ -505,6 +505,9 @@ namespace allscale { namespace components { std::uint64_t schedule_rank = work.id().rank(); + if (schedule_rank == std::uint64_t(-1)) + schedule_rank = rank_; + if ((schedule_rank != rank_ && !work.enqueue_remote()) || !allscale::resilience::rank_running(schedule_rank)) { diff --git a/src/detail/work_item_impl_base.cpp b/src/detail/work_item_impl_base.cpp index b7ecc15..56db42c 100644 --- a/src/detail/work_item_impl_base.cpp +++ b/src/detail/work_item_impl_base.cpp @@ -2,9 +2,9 @@ #include namespace allscale { namespace detail { - void work_item_impl_base::set_this_id(machine_config const& mconfig) + void work_item_impl_base::set_this_id(machine_config const& mconfig, bool is_first) { - id_.set(this, mconfig); + id_.set(this, mconfig, this->can_split() || !is_first); } this_work_item::id const& work_item_impl_base::id() const diff --git a/src/this_work_item.cpp b/src/this_work_item.cpp index e91f113..4d5ca82 100644 --- a/src/this_work_item.cpp +++ b/src/this_work_item.cpp @@ -46,6 +46,11 @@ namespace allscale { namespace this_work_item { next_id_(0) { HPX_ASSERT(i == 0); + reset_distribution(); + } + + void id::reset_distribution() + { config_.rank_ = -1; config_.numa_domain_ = 0; config_.locality_depth_ = -1;//hpx::get_num_localities(hpx::launch::sync); @@ -183,13 +188,13 @@ namespace allscale { namespace this_work_item { void id::update_rank(std::size_t rank) { - HPX_ASSERT(config_.rank_ != std::uint64_t(-1)); +// HPX_ASSERT(config_.rank_ != std::uint64_t(-1)); config_.rank_ = rank; } std::size_t id::rank() const { - HPX_ASSERT(config_.rank_ != std::uint64_t(-1)); +// HPX_ASSERT(config_.rank_ != std::uint64_t(-1)); return config_.rank_; } @@ -204,13 +209,19 @@ namespace allscale { namespace this_work_item { } // void id::set(std::shared_ptr wi) - void id::set(detail::work_item_impl_base* wi, machine_config const& mconfig) + void id::set(detail::work_item_impl_base* wi, machine_config const& mconfig, bool can_split) { id& parent = get_id(); next_id_ = 0; id_ = parent.id_; id_.push_back(parent.next_id_++); + if (!can_split) + { + config_ = parent.config_; + return; + } + // HPX_ASSERT(parent.config_.rank_ != std::uint64_t(-1)); // HPX_ASSERT(parent.config_.locality_depth_ != std::uint64_t(-1)); // HPX_ASSERT(parent.config_.numa_depth_ != std::uint8_t(-1)); @@ -222,12 +233,14 @@ namespace allscale { namespace this_work_item { } else { +// if ((id_.back() & 1) == 0) if (id_.back() == 0) { setup_left(mconfig, parent.config_); } else { +// HPX_ASSERT((id_.back() & 1) == 1); setup_right(mconfig, parent.config_); } } diff --git a/tests/performance/CMakeLists.txt b/tests/performance/CMakeLists.txt index ce6d488..c22db3b 100644 --- a/tests/performance/CMakeLists.txt +++ b/tests/performance/CMakeLists.txt @@ -10,21 +10,21 @@ set(tests data_item_get data_item_get_invariant stream - tpc.allscalecc.latency_n29 - tpc.allscalecc.latency_n30 - tpc.allscalecc.latency_n32 - tpc.allscalecc.latency_n34 - tpc.allscalecc.latency_n36 - tpc.allscalecc.throughput_2_n29 - tpc.allscalecc.throughput_2_n30 - tpc.allscalecc.throughput_2_n32 - tpc.allscalecc.throughput_2_n34 - tpc.allscalecc.throughput_2_n36 - tpc.allscalecc.throughput_n29 - tpc.allscalecc.throughput_n30 - tpc.allscalecc.throughput_n32 - tpc.allscalecc.throughput_n34 - tpc.allscalecc.throughput_n36 + #tpc.allscalecc.latency_n29 + #tpc.allscalecc.latency_n30 + #tpc.allscalecc.latency_n32 + #tpc.allscalecc.latency_n34 + #tpc.allscalecc.latency_n36 + #tpc.allscalecc.throughput_2_n29 + #tpc.allscalecc.throughput_2_n30 + #tpc.allscalecc.throughput_2_n32 + #tpc.allscalecc.throughput_2_n34 + #tpc.allscalecc.throughput_2_n36 + #tpc.allscalecc.throughput_n29 + #tpc.allscalecc.throughput_n30 + #tpc.allscalecc.throughput_n32 + #tpc.allscalecc.throughput_n34 + #tpc.allscalecc.throughput_n36 transpose stencil ) diff --git a/tools/fixup_allscalecc.py b/tools/fixup_allscalecc.py index 8d93a5a..647f49c 100755 --- a/tools/fixup_allscalecc.py +++ b/tools/fixup_allscalecc.py @@ -23,6 +23,8 @@ def fixup(pattern, replace): fixup(r'std::vector<(.*), std::allocator<\1 > >', r'std::vector<\1>') fixup(r'__gnu_cxx::__normal_iterator >', r'std::vector<\1>::const_iterator') fixup(r'__gnu_cxx::__normal_iterator<([^\*]*)\*, std::vector<\1> >', r'std::vector<\1>::iterator') +fixup(r'std::_Rb_tree_iterator]*)> >', r'std::map<\1>::iterator') +fixup(r'std::_Rb_tree_const_iterator]*)> >', r'std::map<\1>::const_iterator') fixup(r'__gnu_cxx::__normal_iterator', r'std::string::const_iterator') fixup(r'__gnu_cxx::__normal_iterator', r'std::string::iterator') fixup(r'(var_[0-9]+)\.operator\*\(\).operator string_type\(\)', r'*\1')