Implemented custom byte_vector for local storage #245
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…com/ryan-dozier/ygm into feature-vector-irecv
steiltre
reviewed
Aug 13, 2024
include/ygm/io/detail/csv.hpp
Outdated
| @@ -18,6 +18,7 @@ class csv_field { | |||
|
|
|||
| bool is_integer() const { | |||
| int64_t test; | |||
| std::cout << m_f << std::endl; | |||
There was a problem hiding this comment.
Looks like a debugging statement was left in
There was a problem hiding this comment.
good catch, removed
include/ygm/detail/comm.ipp
Outdated
| } | ||
|
|
||
| if constexpr (!std::is_empty<Lambda>::value) { | ||
| // oarchive.saveBinary(&l, sizeof(Lambda)); | ||
| size_t size_before = packed.size(); | ||
| packed.resize(size_before + sizeof(Lambda)); | ||
| std::memcpy(packed.data() + size_before, &l, sizeof(Lambda)); | ||
| //packed.push_bytes(&l, sizeof(lambda)); |
There was a problem hiding this comment.
Can this line replace the lines above?
There was a problem hiding this comment.
Yes, there was a typo here having lambda instead of Lambda that I forgot to return to and fix.
| } | ||
|
|
||
| void reserve(size_t cap) { | ||
| size_t new_capacity = get_page_aligned_size(cap); |
There was a problem hiding this comment.
May be worth implementing an exponentially growing size to get the number of calls to mremap to be logarithmic in the buffer size rather than linear.
There was a problem hiding this comment.
Should be experimented with before implementing.
…ter test coverage to the test_byte_vector
… 'ygm/v0.7-dev' into feature-vector-irecv
|
Merging this in after seeing results of scaling out to 128 nodes. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This work aims unify YGM send and recv buffers to the same data type. Previously we were using both
std::vector<std::byte>andstd::array<std::byte>. The goal is to create a sparse, dynamically resizable array. This is needed when many YGM communicators are spawned. This is to avoid fully allocating each of the overly large recv buffers until they are utilized (an issue we found with std::vector).I tried to emulate most of the common std::vector API functions we were using, we should be able to add additional functionality on top of this approach when needed.
I did include a simple test file for CI and unit tests, it may not be fully exhaustive, so reviewing would be good.