-
Notifications
You must be signed in to change notification settings - Fork 44
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.
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
[WIP] Finish Global Dynamic Memory and Allocator Concepts #310
Conversation
…303-glob-dyn-alloc
…at-umap-redesign Conflicts: dash/include/dash/Memory.h dash/include/dash/allocator/DynamicAllocator.h dash/include/dash/memory/MemorySpace.h
47192d3
to
286fa93
Compare
…-303-glob-dyn-alloc Conflicts: dash/include/dash/GlobDynamicMem.h dash/include/dash/GlobMem.h dash/include/dash/Memory.h dash/include/dash/memory/GlobDynamicMem.h dash/include/dash/memory/GlobHeapMem.h dash/include/dash/memory/GlobMem.h dash/include/dash/memory/GlobStaticMem.h dash/include/libdash.h
…h into feat-303-glob-dyn-alloc
Conflicts: dash/include/dash/GlobPtr.h dash/include/dash/map/UnorderedMap.h dash/include/dash/memory/MemorySpace.h dash/test/memory/GlobHeapMemTest.cc
…dyn-alloc Conflicts: dart-if/include/dash/dart/if/dart_globmem.h dash/include/dash/allocator/EpochSynchronizedAllocator.h
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since I am on my way to the airport, I don't have time to fully grasp all the details. Comments inline, good otherwise 👍
{ | ||
return !(*this == rhs); | ||
} | ||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not happy with having additional C++ code in DART. Maybe implement these operators somewhere in DASH, potentially in a header wrapping dart_globmem.h
.
} | ||
|
||
// Move Constructor | ||
memory_block(memory_block &&x) noexcept { *this = std::move(x); } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this .reset()
x
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks quite strange. If this works, better use memory_block(memory_block &&) noexcept = default;
dash/include/dash/map/UnorderedMap.h
Outdated
const_iterator hint, | ||
const value_type & value) | ||
{ | ||
Timer::timestamp_t ts_enter = Timer::Now(), ts_insert, ts_find, d_insert, d_find; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The timestamps are only needed for debugging. They should not be used in builds without debug output.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was actually a commit by accident. Will remove this either. It is something which does not really belong to this pull request.
dash/include/dash/map/UnorderedMap.h
Outdated
/* rkowalewski: | ||
* Why do we increment local size and _local_cumul_size for the corresponding unit at the same time? | ||
* This seems strange to me!! | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this still an issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
no it is not. will remote these comments. :)
|
||
#ifdef DASH_ENABLE_TRACE_LOGGING | ||
std::for_each(std::begin(_num_attach_buckets), | ||
std::end(_num_attach_buckets), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Iterating over num_unattached_buckets
may be more efficient.
* considerations and to optimize communication. | ||
* If the memory space concept complies to the STL memory_resource | ||
* interface, only DART_TYPE_BYTE instead of the actual value type | ||
* is available. This would therefore harm stability and performance. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note: the type argument in DART allocation functions is not used and you can safely pass DART_TYPE_BYTE
to them. It do not affect alignment, which will be taken care of later (DART memory allocation overhaul). In fact, passing the type to the underlying MPI implementation only affects the indexing in access functions (which is in bytes for us, for reasons).
OtherDynAllocTraits::allocator_type otherDynAlloc{dash::Team::All()}; | ||
OtherDynAllocTraits::pointer gp2 = OtherDynAllocTraits::allocate(otherDynAlloc, n); | ||
OtherDynAllocTraits::deallocate(otherDynAlloc, gp2, n); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing newline.
class EpochSynchronizedAllocatorTest : public dash::test::TestBase { | ||
}; | ||
|
||
#endif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No newline.
|
||
/** | ||
* Test fixture for class DASH unit id types. | ||
*/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That comment seems strange ;)
dash/include/dash/map/UnorderedMap.h
Outdated
// Unit mapped to the new element's key by the hash function: | ||
DASH_LOG_TRACE("UnorderedMap.insert", "target unit:", unit); | ||
// No element with specified key exists, insert new value. | ||
ts_insert = Timer::Now(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Benchmarking / debugging leftovers? If these changes are intended to improve performance, we should add a benchmark example.
Update: Not necessary anymore, as already fixed by @devreal While implementing this, please also fix the |
return length != 0 && ptr != nullptr; | ||
} | ||
|
||
void *ptr; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should be encapsulated in std::unique_ptr
, as otherwise the ptr is not set to nullptr
on default move.
Brief (as possible) summary of the concept definitions that emerged from my GDM evaluation for discussion:
Memory Space and Allocator Concepts
Memory Space
The principle task of memory spaces is to maintain a global address space.
This consequently implies the following aspects:
Pointer Semantics
A memory space provides a dependent pointer type that realizes random
access semantics within its address range. Memory space pointers have
two principle concerns:
the address range of their memory space
global pointers
Pointers depend on a memory space instance to provide these operations.
For global address space, pointer types must be defined as
specializations of
dash::GlobPtr
and specified indash::memory_space_traits
as dependent type:using MemSpaceType<T, ...>::pointer = GlobPtr<Tc, MemSpaceType<T, ...>>`
The compatible operations for pointer types of two different memory
spaces is specified by the pointer's implementation.
Resource Interface for Allocators
Allocators depend on memory spaces to attach local memory in the global
address space. In this, memory space types might support access from
multiple allocator instances on the same memory space.
The memory space concept therefore must provide methods to query total
and available capacity.
For example, a static memory space is initialized with a fixed capacity
which might be shared by multiple allocators. Dynamic memory spaces
represent a global heap that automatically grows and shrinks on
allocation and de-allocation.
Synchronization of Distributed Memory Subspaces
Changes on a memory segment usually are not visible at remote locations
immediately. Dynamic memory spaces may, among other aspects, differ in
the communication of memory segment sizes between units (collective epoch
synchronization, deferred point-to-point synchronization, ...) or
data modifications.
For example, a memory space could issue asynchronous pointers that
perform an implicit flush operation (= future) when dereferenced at a
remote address.
Allocator
Allocation Balance
The number of elements allocated at the single units depends on the
allocator, not the memory space type.
More specific, allocator semantics define whether
AllocType.allocate(n)
will reserve
n
elements at every unit (balanced) orn
elements atthe active unit (unbalanced).
An allocator's memory space might support irregular (unbalanced) address
ranges but a balanced allocator may still perform balanced allocation
on it.
Unit Participation in Allocation
Allocator semantics define whether a call of
AllocType.allocate
isa one-sided, collective, or partially collective operation.
Allocators are not responsible for communicating allocated memory
regions across units, however.
For example, a call of
AllocType.allocate(l)
could be unbalanced(local allocation with
l
varying between units) and collective.The propagation of the local sizes
l
between units relies to thememory space.
Traits
Not all combinations of allocator- and memory space types are compatible,
of course. Compatibility can be checked at compile time by matching allocator
traits and memory space traits, just like already implemented in pattern traits.
The same principles used to deduce pattern types from container- and algorithm
constraints can applied to deduce allocator types for memory spaces, or vice
versa, or depending on algorithm execution- and launch policies, etc.
The aspects listed above represent the primary dimensions (= categories) of allocator
and memory space traits.
Interaction
From application perspective, memory can only be acquired from an allocator
instance that is initialized with an underlying memory space.
Memory spaces can again utilize allocators for memory management of their
sub-spaces. A general call dependency is: