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

Support min alignment in ChannelResource #282

Draft
wants to merge 3 commits into
base: dev
Choose a base branch
from
Draft

Support min alignment in ChannelResource #282

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
c27b6ff
Align ascii art border
dennisklein Jun 17, 2020
f6cb292
Support min alignment in ChannelResource
dennisklein Jun 17, 2020
f371a81
Fix -Wunused-result
dennisklein Jun 17, 2020
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
2 changes: 1 addition & 1 deletion .clang-format
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ ColumnLimit: 100
ConstructorInitializerIndentWidth: 4
ContinuationIndentWidth: 4
Cpp11BracedListStyle: true
DerivePointerAlignment: true
DerivePointerAlignment: false
FixNamespaceComments: true
IndentWidth: 4
IndentWrappedFunctionNames: true
Expand Down
3 changes: 2 additions & 1 deletion fairmq/FairMQTransportFactory.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/********************************************************************************
* Copyright (C) 2017-2018 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* Copyright (C) 2017-2020 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* *
* This software is distributed under the terms of the *
* GNU Lesser General Public Licence (LGPL) version 3, *
Expand All @@ -22,6 +22,7 @@ using namespace std;

FairMQTransportFactory::FairMQTransportFactory(const string& id)
: fkId(id)
, fMemoryResource(this)
{}

auto FairMQTransportFactory::CreateTransportFactory(const string& type,
Expand Down
4 changes: 2 additions & 2 deletions fairmq/FairMQTransportFactory.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/********************************************************************************
* Copyright (C) 2014-2018 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* Copyright (C) 2014-2020 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* *
* This software is distributed under the terms of the *
* GNU Lesser General Public Licence (LGPL) version 3, *
Expand Down Expand Up @@ -34,7 +34,7 @@ class FairMQTransportFactory
const std::string fkId;

/// The polymorphic memory resource associated with the transport
fair::mq::ChannelResource fMemoryResource{this};
fair::mq::ChannelResource fMemoryResource;

public:
/// ctor
Expand Down
10 changes: 5 additions & 5 deletions fairmq/MemoryResourceTools.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/********************************************************************************
* Copyright (C) 2018 CERN and copyright holders of ALICE O2 *
* Copyright (C) 2018 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* Copyright (C) 2018 CERN and copyright holders of ALICE O2 *
* Copyright (C) 2018 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* *
* This software is distributed under the terms of the *
* GNU Lesser General Public Licence (LGPL) version 3, *
* copied verbatim in the file "LICENSE" *
* This software is distributed under the terms of the *
* GNU Lesser General Public Licence (LGPL) version 3, *
* copied verbatim in the file "LICENSE" *
********************************************************************************/

/// @brief Tools for interfacing containers to the transport via polymorphic
Expand Down
20 changes: 13 additions & 7 deletions fairmq/MemoryResources.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/********************************************************************************
* Copyright (C) 2018 CERN and copyright holders of ALICE O2 *
* Copyright (C) 2018 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* Copyright (C) 2018 CERN and copyright holders of ALICE O2 *
* Copyright (C) 2018 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* *
* This software is distributed under the terms of the *
* GNU Lesser General Public Licence (LGPL) version 3, *
* copied verbatim in the file "LICENSE" *
* This software is distributed under the terms of the *
* GNU Lesser General Public Licence (LGPL) version 3, *
* copied verbatim in the file "LICENSE" *
********************************************************************************/

/// @brief Memory allocators and interfaces related to managing memory via the
Expand All @@ -15,7 +15,13 @@
#include <fairmq/FairMQTransportFactory.h>
#include <fairmq/MemoryResources.h>

void *fair::mq::ChannelResource::do_allocate(std::size_t bytes, std::size_t alignment)
namespace fair {
namespace mq {

auto createMessage(TransportFactory* factory, std::size_t bytes, Alignment alignment) -> MessagePtr
{
return setMessage(factory->CreateMessage(bytes, fair::mq::Alignment{alignment}));
return factory->CreateMessage(bytes, alignment);
}

} // namespace mq
} // namespace fair
59 changes: 36 additions & 23 deletions fairmq/MemoryResources.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
/********************************************************************************
* Copyright (C) 2018 CERN and copyright holders of ALICE O2 *
* Copyright (C) 2018 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* Copyright (C) 2018 CERN and copyright holders of ALICE O2 *
* Copyright (C) 2018-2020 GSI Helmholtzzentrum fuer Schwerionenforschung GmbH *
* *
* This software is distributed under the terms of the *
* GNU Lesser General Public Licence (LGPL) version 3, *
* copied verbatim in the file "LICENSE" *
* This software is distributed under the terms of the *
* GNU Lesser General Public Licence (LGPL) version 3, *
* copied verbatim in the file "LICENSE" *
********************************************************************************/

/// @brief Memory allocators and interfaces related to managing memory via the
Expand All @@ -30,6 +30,7 @@ namespace mq {

using byte = unsigned char;
namespace pmr = boost::container::pmr;
using TransportFactory = FairMQTransportFactory;

/// All FairMQ related memory resources need to inherit from this interface
/// class for the
Expand All @@ -42,70 +43,82 @@ class FairMQMemoryResource : public pmr::memory_resource
/// buffer), e.g. pointer returned by std::vector::data() return nullptr if
/// returning
/// a message does not make sense!
virtual FairMQMessagePtr getMessage(void *p) = 0;
virtual void *setMessage(FairMQMessagePtr) = 0;
virtual FairMQTransportFactory *getTransportFactory() noexcept = 0;
virtual MessagePtr getMessage(void* p) = 0;
virtual void* setMessage(MessagePtr) = 0;
virtual TransportFactory* getTransportFactory() noexcept = 0;
virtual size_t getNumberOfMessages() const noexcept = 0;
};

auto createMessage(TransportFactory* factory, std::size_t bytes, Alignment alignment) -> MessagePtr;

/// This is the allocator that interfaces to FairMQ memory management. All
/// allocations are
/// delegated to FairMQ so standard (e.g. STL) containers can construct their
/// stuff in
/// memory regions appropriate for the data channel configuration.
class ChannelResource : public FairMQMemoryResource
template<std::size_t MinAlignment>
class AlignedChannelResource : public FairMQMemoryResource
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the name change necessary? Since the default behavior does not change (or does it?), signatures do not change (as far as I can see) it seems to be the same animal, just a better behaved one :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I don't change the name, it would not let me do the name alias: using ChannelResource = ChannelResource<0>; - do you know how to make it work?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

indeed, good question :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

template <int alignment = 0>
struct ChannelResource {
};

no?

{
protected:
FairMQTransportFactory *factory{nullptr};
TransportFactory* factory{nullptr};
// TODO: for now a map to keep track of allocations, something else would
// probably be
// faster, but for now this does not need to be fast.
boost::container::flat_map<void *, FairMQMessagePtr> messageMap;
boost::container::flat_map<void*, MessagePtr> messageMap;

public:
ChannelResource() = delete;
AlignedChannelResource() = delete;

ChannelResource(FairMQTransportFactory *_factory)
AlignedChannelResource(TransportFactory* _factory)
: FairMQMemoryResource()
, factory(_factory)
, messageMap()
{
if (!_factory) {
throw std::runtime_error("Tried to construct from a nullptr FairMQTransportFactory");
}
};
}

FairMQMessagePtr getMessage(void *p) override
MessagePtr getMessage(void* p) override
{
auto mes = std::move(messageMap[p]);
messageMap.erase(p);
return mes;
}

void *setMessage(FairMQMessagePtr message) override
void* setMessage(MessagePtr message) override
{
void *addr = message->GetData();
void* addr = message->GetData();
messageMap[addr] = std::move(message);
return addr;
}

FairMQTransportFactory *getTransportFactory() noexcept override { return factory; }
TransportFactory* getTransportFactory() noexcept override { return factory; }

size_t getNumberOfMessages() const noexcept override { return messageMap.size(); }

protected:
void *do_allocate(std::size_t bytes, std::size_t alignment) override;
void do_deallocate(void *p, std::size_t /*bytes*/, std::size_t /*alignment*/) override
void* do_allocate(std::size_t bytes, std::size_t alignment) override
{
return setMessage(createMessage(factory,
bytes,
alignment < MinAlignment ? Alignment{MinAlignment}
: Alignment{alignment}));
}

void do_deallocate(void* p, std::size_t /*bytes*/, std::size_t /*alignment*/) override
{
messageMap.erase(p);
};
}

bool do_is_equal(const pmr::memory_resource &other) const noexcept override
bool do_is_equal(const pmr::memory_resource& other) const noexcept override
{
return this == &other;
};
}
};

using ChannelResource = AlignedChannelResource<0>;

} /* namespace mq */
} /* namespace fair */

Expand Down
5 changes: 4 additions & 1 deletion fairmq/sdk/DDSEnvironment.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@ struct DDSEnvironment::Impl
" mkdir -p \"$HOME/.DDS\"\n"
" dds-user-defaults --ignore-default-sid -d -c \"$HOME/.DDS/DDS.cfg\"\n"
"fi\n";
std::system(cmd.str().c_str());
auto rc(std::system(cmd.str().c_str()));
if (rc != 0) {
LOG(warn) << "DDSEnvironment::SetupConfigHome failed";
}
}

auto SetupPath() -> void
Expand Down
9 changes: 9 additions & 0 deletions test/memory_resources/_memory_resources.cxx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,4 +137,13 @@ TEST(MemoryResources, getMessage)
EXPECT_TRUE(messageArray[0] == 4 && messageArray[1] == 5 && messageArray[2] == 6);
}

TEST(MemoryResources, AlignedChannelResource)
{
auto factory = TransportFactory::CreateTransportFactory("shmem");
constexpr std::size_t alignment(64);
AlignedChannelResource<alignment> pmr(factory.get());
auto ptr = pmr.allocate(100);
ASSERT_EQ(reinterpret_cast<uintptr_t>(ptr) % 64, 0);
}

} // namespace