Skip to content

Commit

Permalink
Merge pull request #1220 from ApexAI/iox-#1218-use-heap-for-c-binding…
Browse files Browse the repository at this point in the history
…-constructs

Iox #1218 use heap for c binding constructs
  • Loading branch information
elBoberido authored Mar 3, 2022
2 parents 9b24e64 + 302e648 commit 1e62939
Show file tree
Hide file tree
Showing 27 changed files with 279 additions and 329 deletions.
36 changes: 18 additions & 18 deletions .github/workflows/build-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,24 +30,24 @@ jobs:
cmake-version: '3.16.3' # version used in Ubuntu 20.04 LTS
- run: ./tools/ci/build-test-ubuntu.sh

## uses macos to run freebsd in a virtualbox
#build-test-unix-with-freebsd:
## prevent stuck jobs consuming macos runners for 6 hours
#timeout-minutes: 60
#needs: pre-flight-check
#runs-on: macos-10.15
#steps:
#- uses: actions/checkout@v2
#with:
#ref: 'master'
#- name: Unix (FreeBSD) test
#id: Test
#uses: vmactions/[email protected]
#with:
#usesh: true
#mem: 2048
#prepare: pkg install -y cmake git ncurses bash wget bison
#run: ./tools/ci/build-test-freebsd.sh
# uses macos to run freebsd in a virtualbox
build-test-unix-with-freebsd:
# prevent stuck jobs consuming macos runners for 6 hours
timeout-minutes: 60
needs: pre-flight-check
runs-on: macos-10.15
steps:
- uses: actions/checkout@v2
with:
ref: 'master'
- name: Unix (FreeBSD) test
id: Test
uses: vmactions/[email protected]
with:
usesh: true
mem: 2048
prepare: pkg install -y cmake git ncurses bash wget bison
run: ./tools/ci/build-test-freebsd.sh

build-test-windows:
runs-on: windows-2019
Expand Down
1 change: 1 addition & 0 deletions doc/website/release-notes/iceoryx-v2-0-0.md
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,7 @@ fb913bf0de288ba84fe98f7a23d35edfdb22381
- Prevent creation of `popo::Publisher`'s with internal `ServiceDescription` [\#1120](https://github.com/eclipse-iceoryx/iceoryx/issues/1120)
- RelativePointer is now type safe, i.e. can only be constructed from pointers with a valid convertion to the raw pointer [\#1121](https://github.com/eclipse-iceoryx/iceoryx/issues/1121)
- Clamping `historyRequest` to `queueCapacity` [\#1192](https://github.com/eclipse-iceoryx/iceoryx/issues/1192)
- C binding storage sizes do not match for multiple OS's and architectures [\#1218](https://github.com/eclipse-iceoryx/iceoryx/issues/1218)

**Refactoring:**

Expand Down
9 changes: 0 additions & 9 deletions iceoryx_binding_c/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,3 @@ install(
FILES ${CMAKE_CURRENT_SOURCE_DIR}/LICENSE
DESTINATION share/doc/iceoryx_binding_c
COMPONENT dev)

configure_file("${CMAKE_CURRENT_SOURCE_DIR}/cmake/iceoryx_binding_c_deployment.h.in"
"${CMAKE_BINARY_DIR}/generated/iceoryx/include/iceoryx_binding_c/iceoryx_binding_c_deployment.h" @ONLY)

install(
FILES ${CMAKE_BINARY_DIR}/generated/iceoryx/include/${PROJECT_NAME}/iceoryx_binding_c_deployment.h
DESTINATION include/${PREFIX}/${PROJECT_NAME}/
COMPONENT dev
)
26 changes: 0 additions & 26 deletions iceoryx_binding_c/cmake/iceoryx_binding_c_deployment.h.in

This file was deleted.

153 changes: 27 additions & 126 deletions iceoryx_binding_c/include/iceoryx_binding_c/types.h
Original file line number Diff line number Diff line change
Expand Up @@ -18,51 +18,8 @@
#ifndef IOX_BINDING_C_TYPES_H
#define IOX_BINDING_C_TYPES_H

#include "iceoryx_binding_c/iceoryx_binding_c_deployment.h"
#include "internal/c2cpp_binding.h"

/// @brief if the function parameters change due to an update of the listener or
/// waitset recalculate them with the following approach.
/// 1. Run SanityCheck.VerifyStorageSizeCalculationForListener
/// or SanityCheck.VerifyStorageSizeCalculationForWaitSet
/// 2. Take a look at the expected numbers of size 1 (A1) and 2 (A2).
/// 3. Find the parameters m, n for the function StorageSize(x) = m + n * x
/// 4. Re-run the the tests to verify if the parameters are correct.
///
/// Howto calculate those numbers:
/// 1. ./build/binding_c/test/binding_c_moduletests --gtest_filter="SanityCheck.VerifyStorageSizeCalculationForWaitSet"
/// 2. Analyse the output of the failing test:
/// /home/elchris/Development/iceoryx/iceoryx_binding_c/test/moduletests/test_types_storage_size.hpp:77: Failure
/// Value of: sizeof(WaitSet<1>)
/// Expected: is equal to 734
/// Actual: 736 (of type unsigned long)
/// /home/elchris/Development/iceoryx/iceoryx_binding_c/test/moduletests/test_types_storage_size.hpp:78: Failure
/// Value of: sizeof(WaitSet<2>)
/// Expected: is equal to 916
/// Actual: 920 (of type unsigned long)
/// The actual size is for `sizeof(WaitSet<1>)` == 736 and for `sizeof(WaitSet<2>)` == 920
/// 3. With those two values we know StorageSize(1) = m + n * 1 = 736 and StorageSize(2) = m + n * 2 = 920
/// 4. We gain the value of m = 552 and n = 184
///
#if defined(__APPLE__)
#define CALCULATE_STORAGE_SIZE_FOR_LISTENER(numberOfAttachments) \
(144 + numberOfAttachments * 168 - 8 * (((numberOfAttachments + 1) / 2) - 1))
#elif defined(_WIN32)
#define CALCULATE_STORAGE_SIZE_FOR_LISTENER(numberOfAttachments) \
(168 + numberOfAttachments * 192 - 8 * (((numberOfAttachments + 1) / 2) - 1))
#elif defined(__linux__)
#define CALCULATE_STORAGE_SIZE_FOR_LISTENER(numberOfAttachments) (((128 + numberOfAttachments * 140) / 8) * 8)
#elif defined(__FreeBSD__)
#define CALCULATE_STORAGE_SIZE_FOR_LISTENER(numberOfAttachments) \
(88 + numberOfAttachments * 112 - 8 * (((numberOfAttachments + 1) / 2) - 1))
#endif

#if defined(_WIN32)
#define CALCULATE_STORAGE_SIZE_FOR_WAITSET(numberOfAttachments) (552 + numberOfAttachments * 200)
#else
#define CALCULATE_STORAGE_SIZE_FOR_WAITSET(numberOfAttachments) (552 + numberOfAttachments * 184)
#endif

#define IOX_C_CHUNK_DEFAULT_USER_PAYLOAD_ALIGNMENT 8
#define IOX_C_CHUNK_NO_USER_HEADER_SIZE 0
#define IOX_C_CHUNK_NO_USER_HEADER_ALIGNMENT 1
Expand All @@ -74,63 +31,35 @@
/// The size and the alignment of all structs are verified by the
/// binding c integration test iox_types_test

struct iox_ws_storage_t_
typedef struct
{
// the value of the array size is the result of the following formula:
// sizeof(WaitSet) / 8
/// @note see iceoryx_binding_c_deployment.h.in for calculation of the size
uint64_t do_not_touch_me[CALCULATE_STORAGE_SIZE_FOR_WAITSET(IOX_BUILD_GENERATED_MAX_NUMBER_OF_NOTIFIERS) / 8];
};
typedef struct iox_ws_storage_t_ iox_ws_storage_t;
// only size for pointer is necessary
uint64_t do_not_touch_me[1];
} iox_ws_storage_t;

struct iox_user_trigger_storage_t_
typedef struct
{
// the value of the array size is the result of the following formula:
// sizeof(UserTrigger) / 8
#if defined(__APPLE__)
uint64_t do_not_touch_me[15];
#elif defined(_WIN32)
uint64_t do_not_touch_me[18];
#elif defined(__FreeBSD__)
uint64_t do_not_touch_me[8];
#elif defined(__linux__)
uint64_t do_not_touch_me[12];
#endif
};
typedef struct iox_user_trigger_storage_t_ iox_user_trigger_storage_t;
// only size for pointer is necessary
uint64_t do_not_touch_me[1];
} iox_user_trigger_storage_t;

struct iox_sub_storage_t_
typedef struct
{
// the value of the array size is the result of the following formula:
// sizeof(cpp2c_Subscriber) / 8
#if defined(__APPLE__)
uint64_t do_not_touch_me[16];
#elif defined(_WIN32)
uint64_t do_not_touch_me[19];
#elif defined(__FreeBSD__)
uint64_t do_not_touch_me[9];
#elif defined(__linux__)
uint64_t do_not_touch_me[13];
#endif
};
typedef struct iox_sub_storage_t_ iox_sub_storage_t;
// only size for pointer is necessary
uint64_t do_not_touch_me[1];
} iox_sub_storage_t;

struct iox_pub_storage_t_
typedef struct
{
// the value of the array size is the result of the following formula:
// sizeof(cpp2c_Publisher) / 8
// only size for pointer is necessary
uint64_t do_not_touch_me[1];
};
typedef struct iox_pub_storage_t_ iox_pub_storage_t;
} iox_pub_storage_t;

struct iox_listener_storage_t_
typedef struct
{
// the value of the array size is the result of the following formula:
// sizeof(Listener) / 8
/// @note see iceoryx_binding_c_deployment.h.in for calculation of the size
uint64_t do_not_touch_me[CALCULATE_STORAGE_SIZE_FOR_LISTENER(IOX_BUILD_GENERATED_MAX_NUMBER_OF_NOTIFIERS) / 8];
};
typedef struct iox_listener_storage_t_ iox_listener_storage_t;
// only size for pointer is necessary
uint64_t do_not_touch_me[1];
} iox_listener_storage_t;

/// @brief handle of the chunk header
typedef struct
Expand All @@ -142,50 +71,22 @@ typedef struct
/// @brief has exactly the size required to store the underlying object of iox_client_t
typedef struct
{
// the value of the array size is the result of the following formula:
// sizeof(UntypedClient) / 8
#if defined(__APPLE__)
uint64_t do_not_touch_me[22];
#elif defined(_WIN32)
uint64_t do_not_touch_me[25];
#elif defined(__FreeBSD__)
uint64_t do_not_touch_me[15];
#elif defined(__linux__)
uint64_t do_not_touch_me[19];
#endif
// only size for pointer is necessary
uint64_t do_not_touch_me[1];
} iox_client_storage_t;

/// @brief has exactly the size required to store the underlying object of iox_server_t
typedef struct
{
// the value of the array size is the result of the following formula:
// sizeof(UntypedServer) / 8
#if defined(__APPLE__)
uint64_t do_not_touch_me[22];
#elif defined(_WIN32)
uint64_t do_not_touch_me[25];
#elif defined(__FreeBSD__)
uint64_t do_not_touch_me[15];
#elif defined(__linux__)
uint64_t do_not_touch_me[19];
#endif
// only size for pointer is necessary
uint64_t do_not_touch_me[1];
} iox_server_storage_t;

/// @brief has exactly the size required to store the underlying object of iox_service_discovery_t
struct iox_service_discovery_storage_t
typedef struct
{
// the value of the array size is the result of the following formula:
// sizeof(ServiceDiscovery) / 8
#if defined(__APPLE__)
uint64_t do_not_touch_me[30];
#elif defined(_WIN32)
uint64_t do_not_touch_me[35];
#elif defined(__FreeBSD__)
uint64_t do_not_touch_me[16];
#elif defined(__linux__)
uint64_t do_not_touch_me[24];
#endif
};
typedef struct iox_service_discovery_storage_t iox_service_discovery_storage_t;
// only size for pointer is necessary
uint64_t do_not_touch_me[1];
} iox_service_discovery_storage_t;

#endif
13 changes: 7 additions & 6 deletions iceoryx_binding_c/source/c_client.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -77,19 +77,20 @@ iox_client_t iox_client_init(iox_client_storage_t* self,
clientOptions.serverTooSlowPolicy = c2cpp::consumerTooSlowPolicy(options->serverTooSlowPolicy);
}

new (self) UntypedClient(ServiceDescription{IdString_t(TruncateToCapacity, service),
IdString_t(TruncateToCapacity, instance),
IdString_t(TruncateToCapacity, event)},
clientOptions);
auto* me = new UntypedClient(ServiceDescription{IdString_t(TruncateToCapacity, service),
IdString_t(TruncateToCapacity, instance),
IdString_t(TruncateToCapacity, event)},
clientOptions);

return reinterpret_cast<iox_client_t>(self);
self->do_not_touch_me[0] = reinterpret_cast<uint64_t>(me);
return me;
}

void iox_client_deinit(iox_client_t const self)
{
iox::cxx::Expects(self != nullptr);

self->~UntypedClient();
delete self;
}

iox_AllocationResult iox_client_loan_request(iox_client_t const self, void** const payload, const uint32_t payloadSize)
Expand Down
7 changes: 4 additions & 3 deletions iceoryx_binding_c/source/c_listener.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,16 @@ iox_listener_t iox_listener_init(iox_listener_storage_t* self)
{
iox::cxx::Expects(self != nullptr);

auto me = new (self) Listener();
return reinterpret_cast<iox_listener_t>(me);
auto* me = new Listener();
self->do_not_touch_me[0] = reinterpret_cast<uint64_t>(me);
return me;
}

void iox_listener_deinit(iox_listener_t const self)
{
iox::cxx::Expects(self != nullptr);

self->~Listener();
delete self;
}

ENUM iox_ListenerResult iox_listener_attach_subscriber_event(iox_listener_t const self,
Expand Down
10 changes: 6 additions & 4 deletions iceoryx_binding_c/source/c_publisher.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ iox_pub_t iox_pub_init(iox_pub_storage_t* self,
return nullptr;
}

new (self) cpp2c_Publisher();
iox_pub_t me = reinterpret_cast<iox_pub_t>(self);

PublisherOptions publisherOptions;

// use default options otherwise
Expand All @@ -93,6 +90,9 @@ iox_pub_t iox_pub_init(iox_pub_storage_t* self,
publisherOptions.subscriberTooSlowPolicy = c2cpp::consumerTooSlowPolicy(options->subscriberTooSlowPolicy);
}

auto* me = new cpp2c_Publisher();
self->do_not_touch_me[0] = reinterpret_cast<uint64_t>(me);

me->m_portData = PoshRuntime::getInstance().getMiddlewarePublisher(
ServiceDescription{
IdString_t(TruncateToCapacity, service),
Expand All @@ -105,8 +105,10 @@ iox_pub_t iox_pub_init(iox_pub_storage_t* self,

void iox_pub_deinit(iox_pub_t const self)
{
iox::cxx::Expects(self != nullptr);

self->m_portData->m_toBeDestroyed.store(true, std::memory_order_relaxed);
self->~cpp2c_Publisher();
delete self;
}

iox_AllocationResult iox_pub_loan_chunk(iox_pub_t const self, void** const userPayload, const uint32_t userPayloadSize)
Expand Down
13 changes: 7 additions & 6 deletions iceoryx_binding_c/source/c_server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -75,19 +75,20 @@ iox_server_t iox_server_init(iox_server_storage_t* self,
serverOptions.clientTooSlowPolicy = c2cpp::consumerTooSlowPolicy(options->clientTooSlowPolicy);
}

new (self) UntypedServer(ServiceDescription{IdString_t(TruncateToCapacity, service),
IdString_t(TruncateToCapacity, instance),
IdString_t(TruncateToCapacity, event)},
serverOptions);
auto* me = new UntypedServer(ServiceDescription{IdString_t(TruncateToCapacity, service),
IdString_t(TruncateToCapacity, instance),
IdString_t(TruncateToCapacity, event)},
serverOptions);

return reinterpret_cast<iox_server_t>(self);
self->do_not_touch_me[0] = reinterpret_cast<uint64_t>(me);
return me;
}

void iox_server_deinit(iox_server_t const self)
{
iox::cxx::Expects(self != nullptr);

self->~UntypedServer();
delete self;
}

iox_ServerRequestResult iox_server_take_request(iox_server_t const self, const void** const payload)
Expand Down
Loading

0 comments on commit 1e62939

Please sign in to comment.