[resolve #18] Add C++ allocator (partial) #28
Closed
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
added 2 commits
April 3, 2023 03:58
- An error occurred while linking the `RdmaAllocator` library to the `warm_benchmark` program. ``` FAILED: benchmarks/warm_benchmarker : && /bin/clang++-15 -Wall -Wextra -g -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_DEBUG CMakeFiles/warm_benchmarker.dir/benchmarks/warm_benchmark.cpp.o CMakeFiles/warm_benchmarker.dir/benchmarks/warm_benchmark_opts.cpp.o -o benchmarks/warm_benchmarker _deps/spdlog-build/libspdlogd.a librfaaslib.a libbenchmarks.a librfaaslib.a librdmalib.a _deps/spdlog-build/libspdlogd.a /usr/lib/x86_64-linux-gnu/librdmacm.so /usr/lib/x86_64-linux-gnu/libibverbs.so -ldl && : /bin/ld: /bin/ld: DWARF error: invalid or unhandled FORM value: 0x23 CMakeFiles/warm_benchmarker.dir/benchmarks/warm_benchmark.cpp.o: in function `main': warm_benchmark.cpp:(.text+0x493): undefined reference to `rfaas::RdmaAllocator<rdmalib::Buffer<char> >::allocate(unsigned long const&, int const&, unsigned long)' /bin/ld: warm_benchmark.cpp:(.text+0x4e0): undefined reference to `rfaas::RdmaAllocator<rdmalib::Buffer<char> >::allocate(unsigned long const&, int const&, unsigned long)' clang: error: linker command failed with exit code 1 (use -v to see invocation) ninja: build stopped: subcommand failed. ``` - Checking the argument and parameter types helped resolve the linking error. - Inline functions are recommended to be merged into header files to allow for their optimization by the compiler.
- Compiled successfully without any errors. - Add allocator implementation in rfaaslib. - Encapsulate the memory registration in rdmalib - Add test demonstrating standard memory allocation.
mcopik
requested changes
Apr 3, 2023
CMakeLists.txt
Outdated
| @@ -22,7 +22,7 @@ set(CMAKE_CXX_STANDARD_REQUIRED TRUE) | |||
| string(REPLACE "-DNDEBUG" "" CMAKE_CXX_FLAGS_RELWITHDEBINFO "${CMAKE_CXX_FLAGS_RELWITHDEBINFO}") | |||
| string(APPEND CMAKE_CXX_FLAGS_RELWITHDEBINFO " -g -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_DEBUG ") | |||
| string(APPEND CMAKE_CXX_FLAGS_DEBUG " -DSPDLOG_ACTIVE_LEVEL=SPDLOG_LEVEL_DEBUG ") | |||
| string(APPEND CMAKE_CXX_FLAGS " -Wall -Wextra ") | |||
| string(APPEND CMAKE_CXX_FLAGS " -Wall -Wextra -std=c++17") | |||
There was a problem hiding this comment.
We should use CXX_STANDARD instead.
| #include <rfaas/executor.hpp> | ||
|
|
||
| namespace rfaas { | ||
| template<typename T> |
There was a problem hiding this comment.
I think the allocator should be of type T, and then always return rdmalib::Buffer object. If you look at the C++ allocator concept, then I think we should use A::pointer as rdmalib::Buffer.
| if (size > std::size_t(-1) / sizeof(T)) | ||
| throw std::bad_alloc(); | ||
|
|
||
| auto buffer = new rdmalib::Buffer<char>(size, header); |
There was a problem hiding this comment.
I don't think we need dynamic allocation for the buffer object itself :-) Once we properly define the pointer type.
… requirements example for the Allocator"](https://en.cppreference.com/w/cpp/named_req/Allocator). Signed-off-by: mou <William-Mou>
|
Hi @mcopik, |
|
@William-Mou Thanks! I will try to test this and merge in the coming days :) |
5 tasks
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.
Story
This PR is open for issue #18, including the following features.
Achievements
rdma_allocator.hpp, which provides the allocate and deallocate functions.rfaas::RdmaAllocator<rdmalib::Buffer<char> > rdmaAllocator(executor);to instantiate the allocator object.rdmaAllocator.allocate(size_t size, int access)intuitively allocates memory space for a buffer.Test
std::coutin the allocator, you can test the program (eg:warm_benchmarker) to run as expected.TODO