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.

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

[Resolve #18] Add C++ allocator #30

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

William-Mou
Copy link
Contributor

@William-Mou William-Mou commented Apr 6, 2023

Story

This PR is open for issue #18, including the following features.
I have rewritten a better version to comply with the C++ standard.

  • Add allocator implementation in rfaaslib.
  • Encapsulate the memory registration in rdmalib - provide generic enums for local/remote read/write ops + atomics.
  • Support in allocator registration and deregistration of memory.
  • Add test demonstrating standard memory allocation.
  • Add test demonstrating allocation with std::vector and our custom allocator.

Achievements

  • Defines rdma_allocator.hpp, which provides the allocate and deallocate functions.
  • Use rfaas::RdmaAllocator<rdmalib::Buffer<char>> allocator_in{info_in}; to instantiate the allocator object.
  • allocator_in.allocate(opts.input_size); intuitively allocates memory space for a buffer.

Test

  • By inserting report in the allocator, you can test the program (eg: warm_benchmarker ) to run as expected.
warm_benchmarker --config benchmark.json --device-database client_devices.json --name empty --functions /home/mou/rFaaS/build-test/examples/libfunctions.so --executors-database executors_database.json -s 0xFFF

mou added 4 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.
… requirements example for the Allocator"](https://en.cppreference.com/w/cpp/named_req/Allocator).

Signed-off-by: mou <William-Mou>
@William-Mou
Copy link
Contributor Author

William-Mou commented Apr 6, 2023

Hi @mcopik ,

I've closed PR #28.
Here is the complete C++ allocator version that accomplishes all of the required tasks.

- Add a `construct` method in `rdmaAllocator.hpp` to enable allocation with std::vector.
- Test demonstrating standard memory allocation practices.
- Test demonstrating allocation with our custom allocator.
- Test demonstrating allocation with std::vector.
- Improve coding style and adhering to clang-tidy standards.

Signed-off-by: mou <William-Mou>
@mcopik
Copy link
Contributor

mcopik commented Apr 21, 2023

@William-Mou Thanks, much appreciated! I like the idea of passing extra information, such as the pointer, to the protection domain (PD) via additional structure - much cleaner and possible to extend with libfabric.

I'm a bit confused with one part - when you create an allocator of type rdmalib::Buffer<char> and then do static_cast<T *>, how does it work exactly? The thing is that a buffer is a structure containing a pointer to the actual memory allocation.

@William-Mou
Copy link
Contributor Author

Hi @mcopik,

Thank you for your kind words of affirmation!
Regarding the code you mentioned, here is my explanation:

Sample: test demonstrating allocation with our custom allocator.

In the second example (line 80 to line 90 in benchmarks/warm_benchmark.cpp), the RdmaAllocator is instantiated with a type rdmalib::Buffer<char> as the template parameter. When allocator_in.allocate() is called, it returns a void* by std::malloc(size * sizeof(T) + _info.header_size)), which is then cast to a pointer of type T* through a static_cast, where T is rdmalib::Buffer<char>. This means the returned void* pointer is being cast to a pointer of type rdmalib::Buffer<char>*.

Since the allocator creates and returns a rdmalib::Buffer<char>*, the construct() function can then call p->register_memory(), where p is the pointer to the rdmalib::Buffer<char> object, and register_memory() is a member function of rdmalib::Buffer<char> that takes an ibv_pd* pointer and an integer access flag as arguments.

Sample: test demonstrating allocation with std::vector.

Similarly, in the third example (line 93 to line 10 in benchmarks/warm_benchmark.cpp), the C++ vector calls the allocate and construct member functions of the RdmaAllocator, which is passed as a template argument to the vector implicitly. When a new element is added to the vector, the vector calls the allocate function to allocate memory for the new element and then calls the construct function to construct the new element in the allocated memory. When an element is removed from the vector, the vector calls the destructor of the element and then calls the deallocate function to deallocate the memory used by the element.

The above explains how auto p = static_cast<T *>(std::malloc(size * sizeof(T) + _info.header_size)) works and is called. Thank you!

@William-Mou
Copy link
Contributor Author

William-Mou commented Apr 22, 2023

Hi @mcopik,
I have one uncertainty about how creating rdmalib::Buffer<char> elements using a vector.

  rfaas::RdmaInfo info_v_in(executor, IBV_ACCESS_LOCAL_WRITE, rdmalib::functions::Submission::DATA_HEADER_SIZE);
  rfaas::RdmaAllocator<rdmalib::Buffer<char>> allocator_v_in{info_v_in};
  std::vector<rdmalib::Buffer<char>, rfaas::RdmaAllocator<rdmalib::Buffer<char>>> v_in(allocator_v_in);

Does each element in the vector need to allocate memory for the header? The current implementation is that each element is a complete Buffer<char>, and the use case is to store multiple sets of rdmalib::Buffer<char>(maybe a set of in or out) in the vector simultaneously.

@mcopik mcopik mentioned this pull request Mar 7, 2024
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants