Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bug/internal/sorter/memory usage #685

Merged
merged 3 commits into from
Jul 1, 2024
Merged

Bug/internal/sorter/memory usage #685

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
810442f
Update comments in <adiar/internal/data_structures/sorter.h> to 100
SSoelvsten Jul 1, 2024
291d683
Fix TPIE (external) sorter does not have enough memory for 'number_of…
SSoelvsten Jul 1, 2024
7210309
Fix warning on comparison of signed and unsigned (Thanks, Clang)
SSoelvsten Jul 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
89 changes: 43 additions & 46 deletions src/adiar/internal/data_structures/sorter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ namespace adiar::internal
template <memory_mode mem_mode, typename T, typename Comp = std::less<T>>
class sorter;

//////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////
/// \brief Wrapper for TPIE's internal vector with standard quick-sort.
//////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////
template <typename T, typename Comp>
class sorter<memory_mode::Internal, T, Comp>
{
Expand Down Expand Up @@ -144,23 +144,21 @@ namespace adiar::internal
}
};

//////////////////////////////////////////////////////////////////////////////
/// \brief Type alias for sorter for partial type application of the
/// 'internal' memory type.
//////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////
/// \brief Type alias for sorter for partial type application of the 'internal' memory type.
//////////////////////////////////////////////////////////////////////////////////////////////////
template <typename T, typename Comp = std::less<T>>
using internal_sorter = sorter<memory_mode::Internal, T, Comp>;

// LCOV_EXCL_START
// TODO: Unit test external memory variants?

//////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////
/// \brief Wrapper for TPIE's external memory sorter, tpie::merge_sorter.
///
/// A wrapper for the tpie::merge_sorter that takes care of all the memory
/// computations involved in deriving how much memory should be used and how
/// it should be.
//////////////////////////////////////////////////////////////////////////////
/// A wrapper for the tpie::merge_sorter that takes care of all the memory computations involved
/// in deriving how much memory should be used and how it should be.
//////////////////////////////////////////////////////////////////////////////////////////////////
template <typename T, typename Comp>
class sorter<memory_mode::External, T, Comp>
{
Expand Down Expand Up @@ -199,7 +197,11 @@ namespace adiar::internal
sorter(size_t memory_bytes, size_t no_elements, size_t number_of_sorters, Comp comp = Comp())
: _sorter(comp)
{
// =======================================================================
// Quickfix: Issue https://github.com/thomasmoelhave/tpie/issues/250
constexpr tpie::memory_size_type minimum_phase1 =
sizeof(value_type) * 128 * 1024 + 5 * 1024 * 1024;

// ===========================================================================================
// Case 0: No sorters - why are we then instantiating one?
adiar_assert(number_of_sorters > 0, "Number of sorters should be positive");

Expand All @@ -209,53 +211,49 @@ namespace adiar::internal
const tpie::memory_size_type no_elements_memory =
2 * sorter<memory_mode::Internal, value_type, Comp>::memory_usage(no_elements);

// =======================================================================
// ===========================================================================================
// Case 1: A single sorter.
if (number_of_sorters == 1u) {
const size_t sorter_memory = std::min(no_elements_memory, memory_bytes);
const size_t sorter_memory =
std::min(std::max(no_elements_memory, 2 * minimum_phase1), memory_bytes);

adiar_assert(sorter_memory <= memory_bytes,
"Memory of a single sorter does not exceed given amount.");

// ---------------------------------------------------------------------
// -----------------------------------------------------------------------------------------
// Use TPIE's default settings for the three phases.
_sorter.set_available_memory(sorter_memory);
_sorter.begin();
return;
}

// ======================================================================
// ==========================================================================================
// Case: 2+: Distribute a common area of memory between all sorters.
adiar_assert(number_of_sorters > 1, "Edge case for a single sorter taken care of above");

// -----------------------------------------------------------------------
// We intend to have the memory divided between all the sorters, such that
// one can be in phase 2 while everyone else is in phase 1 or 3.
// -------------------------------------------------------------------------------------------
// We intend to have the memory divided between all the sorters, such that one can be in phase
// 2 while everyone else is in phase 1 or 3.
//
// | p1 | p1 | p1 | . . . p2 . . . |
//
// Phase two is the one that makes the most out of a lot of memory. So, we
// want to have phase 2 have the largest share. For simplicity, we
// currently have all the p1 sorters have 1/16 of the entire memory.
// Phase two is the one that makes the most out of a lot of memory. So, we want to have phase
// 2 have the largest share. For simplicity, we currently have all the p1 sorters have 1/16 of
// the entire memory.
//
// TODO: It would be better to make p2 'exponentially' larger than p1 for
// some notion of 'exponentiality'.
// TODO: It would be better to make p2 'exponentially' larger than p1 for some notion of
// 'exponentiality'.
//
// TODO: phase 1 should be upper bounded by the number of elements
// possibly placed in this queue. See Issue #189 on ssoelvsten/adiar
// as to why. I would suggest for us to upper bound the 1/16th by
// slightly more than the amount of memory necessary to hold all
// TODO: phase 1 should be upper bounded by the number of elements possibly placed in this
// queue. See Issue #189 on ssoelvsten/adiar as to why. I would suggest for us to upper
// bound the 1/16th by slightly more than the amount of memory necessary to hold all
// values simultaneously.

// -----------------------------------------------------------------------
// -------------------------------------------------------------------------------------------
// Phase 1 : Push Mergesort base cases
//
// Quickfix: Issue https://github.com/thomasmoelhave/tpie/issues/250
constexpr tpie::memory_size_type minimum_phase1 =
sizeof(value_type) * 128 * 1024 + 5 * 1024 * 1024;

// Take up at most 1/(Sorter-1)'th of 1/16th of the total memory. The last
// sorter is either in the same phase or another phase.
// Take up at most 1/(Sorter-1)'th of 1/16th of the total memory. The last sorter is either in
// the same phase or another phase.
//
// Intendeds shared memory layout of sorters
// +--------+-----------------------------------------------------+
Expand All @@ -266,20 +264,20 @@ namespace adiar::internal
const tpie::memory_size_type phase1 =
std::max(minimum_phase1, std::min(maximum_phase1, no_elements_memory));

// -----------------------------------------------------------------------
// -------------------------------------------------------------------------------------------
// Phase 3 : Top-most and final merging of partial lists.
//
// Based on internal workings of `tpie::merge_sorter` this should be at
// least twice the size of the phase 1 memory.
// Based on internal workings of `tpie::merge_sorter` this should be at least twice the size
// of the phase 1 memory.
constexpr tpie::memory_size_type minimum_phase3 = /*2 **/ minimum_phase1;

const tpie::memory_size_type phase3 = std::max(minimum_phase3, phase1);

// -----------------------------------------------------------------------
// -------------------------------------------------------------------------------------------
// Phase 2 : Merge sorted lists until there are few enough for phase 3.
//
// Use the remaining sorter memory for this very step. If one is in this
// phase, then all other sorters are in phase 1.
// Use the remaining sorter memory for this very step. If one is in this phase, then all other
// sorters are in phase 1.
const tpie::memory_size_type phase2 = memory_bytes - phase1 * (number_of_sorters - 1);

// -----------------------------------------------------------------------
Expand All @@ -296,7 +294,7 @@ namespace adiar::internal
adiar_assert((number_of_sorters - 1) * phase1 + phase3 + phase1 <= memory_bytes,
"Memory is enough to replace the pullable sorter with an 'Nth' pushable one.");

// -----------------------------------------------------------------------
// -------------------------------------------------------------------------------------------
// Set the available memory and start the sorter
_sorter.set_available_memory(phase1, phase2, phase3);
_sorter.begin();
Expand Down Expand Up @@ -366,10 +364,9 @@ namespace adiar::internal
}
};

//////////////////////////////////////////////////////////////////////////////
/// \brief Type alias for sorter for partial type application of the
/// 'external' memory type.
//////////////////////////////////////////////////////////////////////////////
//////////////////////////////////////////////////////////////////////////////////////////////////
/// \brief Type alias for sorter for partial type application of the 'external' memory type.
//////////////////////////////////////////////////////////////////////////////////////////////////
template <typename T, typename Comp = std::less<T>>
using external_sorter = sorter<memory_mode::External, T, Comp>;

Expand Down
2 changes: 1 addition & 1 deletion src/adiar/internal/data_types/level_info.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ namespace adiar::internal
using level_type = level_info::level_type;
using signed_level_type = level_info::signed_level_type;

adiar_assert(levels < 0 ? (std::abs(levels) <= li.level())
adiar_assert(levels < 0 ? (static_cast<level_type>(std::abs(levels)) <= li.level())
: (li.level() + levels <= ptr_uint64::max_label));

// TODO: Use dynamic casts instead?
Expand Down
Loading