Skip to content

[WIP] Fix symbol visibility for user extensions #386

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

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

[WIP] Fix symbol visibility for user extensions #386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
1c617e9
Consolidate csp-autogen impls, restore binding_int_t in platform, fix…
timkpaine Oct 2, 2024
49a7bfa
Make all public classes of CSP visible so that user extensions can ac…
AdamGlustein Oct 2, 2024
73e106f
Merge branch 'tkp/cpp' of https://github.com/Point72/csp into fix-sym…
AdamGlustein Oct 7, 2024
5414d97
Remove CSP_PUBLIC from SRMWLockFreeQueue
AdamGlustein Oct 7, 2024
571b9d5
Merge branch 'main' of https://github.com/Point72/csp into fix-symbol…
AdamGlustein Nov 8, 2024
512a6fd
Use cmake GenerateExportHeader to hopefully fix Windows visibility as…
AdamGlustein Nov 8, 2024
a284577
Build csp core and csp engine as DLLs
AdamGlustein Nov 8, 2024
eb659d8
Append to RPATH for tests to find CSP shared libs
AdamGlustein Nov 11, 2024
9168324
Don't export header-only classes
AdamGlustein Nov 11, 2024
4d826a6
Make Exception and KafkaAdapterManager public vis
AdamGlustein Nov 11, 2024
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
6 changes: 3 additions & 3 deletions .github/workflows/conda.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,9 +32,9 @@ jobs:
strategy:
matrix:
os:
- ubuntu-24.04
- macos-14
- macos-12
# - ubuntu-24.04
# - macos-14
# - macos-12
- windows-2019
runs-on: ${{ matrix.os }}
steps:
Expand Down
2 changes: 2 additions & 0 deletions cpp/csp/adapters/kafka/KafkaAdapterManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

#include <csp/core/Enum.h>
#include <csp/core/Hash.h>
#include <csp/core/Platform.h>
#include <csp/engine/AdapterManager.h>
#include <csp/engine/Dictionary.h>
#include <csp/engine/PushInputAdapter.h>
Expand All @@ -11,6 +12,7 @@
#include <unordered_map>
#include <vector>


namespace RdKafka
{

Expand Down
5 changes: 3 additions & 2 deletions cpp/csp/adapters/parquet/DialectGenericListReaderInterface.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,15 @@
#define _IN_CSP_ADAPTERS_PARQUET_DialectGenericListReaderInterface_H

#include <memory>
#include <csp/core/Platform.h>
#include <csp/engine/DialectGenericType.h>
#include <csp/adapters/parquet/ParquetReaderColumnAdapter.h>
#include <arrow/array/builder_base.h>

namespace csp::adapters::parquet
{

class DialectGenericListReaderInterface
class CSPPARQUETADAPTERIMPL_EXPORT DialectGenericListReaderInterface
{
public:
using Ptr = std::shared_ptr<DialectGenericListReaderInterface>;
Expand All @@ -25,7 +26,7 @@ class DialectGenericListReaderInterface
};

template< typename T >
class CSP_PUBLIC TypedDialectGenericListReaderInterface : public DialectGenericListReaderInterface
class CSPPARQUETADAPTERIMPL_EXPORT TypedDialectGenericListReaderInterface : public DialectGenericListReaderInterface
{
public:
using Ptr = std::shared_ptr<TypedDialectGenericListReaderInterface<T>>;
Expand Down
3 changes: 2 additions & 1 deletion cpp/csp/adapters/parquet/ParquetInputAdapterManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <csp/adapters/parquet/ParquetReader.h>
#include <csp/adapters/utils/StructAdapterInfo.h>
#include <csp/core/Generator.h>
#include <csp/core/Platform.h>
#include <csp/engine/AdapterManager.h>
#include <csp/engine/Dictionary.h>
#include <unordered_map>
Expand All @@ -18,7 +19,7 @@ namespace csp::adapters::parquet


//Top level AdapterManager object for all parquet adapters in the engine
class CSP_PUBLIC ParquetInputAdapterManager final : public csp::AdapterManager
class CSPPARQUETADAPTERIMPL_EXPORT ParquetInputAdapterManager final : public csp::AdapterManager
{
public:
using GeneratorPtr = csp::Generator<std::string, csp::DateTime, csp::DateTime>::Ptr;
Expand Down
3 changes: 2 additions & 1 deletion cpp/csp/adapters/parquet/ParquetOutputAdapterManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <csp/adapters/parquet/ParquetReader.h>
#include <csp/adapters/utils/StructAdapterInfo.h>
#include <csp/core/Generator.h>
#include <csp/core/Platform.h>
#include <csp/engine/AdapterManager.h>
#include <csp/engine/Dictionary.h>
#include <set>
Expand All @@ -21,7 +22,7 @@ class ParquetOutputFilenameAdapter;
class ParquetDictBasketOutputWriter;

//Top level AdapterManager object for all parquet adapters in the engine
class CSP_PUBLIC ParquetOutputAdapterManager final : public csp::AdapterManager
class CSPPARQUETADAPTERIMPL_EXPORT ParquetOutputAdapterManager final : public csp::AdapterManager
{
public:
using FileVisitorCallback = std::function<void(const std::string &)>;
Expand Down
2 changes: 1 addition & 1 deletion cpp/csp/adapters/websocket/ClientAdapterManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ struct WebsocketClientStatusTypeTraits

using ClientStatusType = Enum<WebsocketClientStatusTypeTraits>;

class CSP_PUBLIC ClientAdapterManager final : public AdapterManager
class CSPWEBSOCKETADAPTERIMPL_EXPORT ClientAdapterManager final : public AdapterManager
{
public:
ClientAdapterManager(
Expand Down
9 changes: 8 additions & 1 deletion cpp/csp/core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
include(GenerateExportHeader)

set(CORE_PUBLIC_HEADERS
BasicAllocator.h
Config.h.in
Expand Down Expand Up @@ -25,7 +27,11 @@ set(CORE_SOURCE_FILES

configure_file(Config.h.in ${CMAKE_CURRENT_BINARY_DIR}/cpp/csp/core/Config.h)

add_library(csp_core STATIC ${CORE_SOURCE_FILES})
Copy link
Collaborator

Choose a reason for hiding this comment

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

did you intentionally change from static to shared here?

Copy link
Collaborator

Choose a reason for hiding this comment

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

yeah, the issue was the test binaries link against csp_core directly. as-is, the static build of csp_core is built without cspimpl export macro, so the classes are marked as dllimport which results in failed imports.

Adam and I discussed a few solutions, one of which is just making csp_core shared and creating its own export macro. another is to continue to build csp_core statically (with its own export macro, cmake's export macro for static libs is a no-op, or none at all), and have users link to that directly instead of cspimpl to get at classes like DateTime. basically need to have exactly one dllexport of each class in any .dll in one way or another if it is ever marked dllimport

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yeah, as Sorin explained this is so csp_core / csp_engine can individually export their symbols for tests (or any C++ binary that links them in without cspimpl).
I had tried to export most of these symbols in cspimpl but that approach did not work for the above reason.

add_library(csp_core SHARED ${CORE_SOURCE_FILES})
generate_export_header(csp_core
BASE_NAME cspcore
EXPORT_MACRO_NAME CSPCORE_EXPORT
)
set_target_properties(csp_core PROPERTIES PUBLIC_HEADER "${CORE_PUBLIC_HEADERS}")

install(TARGETS csp_core
Expand All @@ -34,3 +40,4 @@ install(TARGETS csp_core
LIBRARY DESTINATION lib/
)
install(FILES ${CMAKE_CURRENT_BINARY_DIR}/cpp/csp/core/Config.h DESTINATION include/csp/core)
install(FILES ${CMAKE_CURRENT_BINARY_DIR}/cspcore_export.h DESTINATION include/csp/core)
2 changes: 1 addition & 1 deletion cpp/csp/core/Enum.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ bool UnknownOnInvalidValue(long) { return false; }

START_PACKED
template<typename EnumTraits>
struct Enum : public EnumTraits
struct CSP_PUBLIC Enum : public EnumTraits // need to export for autogen build
{
using EnumV = typename EnumTraits::_enum;
using Mapping = std::vector<std::string>;
Expand Down
2 changes: 2 additions & 0 deletions cpp/csp/core/EnumBitSet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
#ifndef _IN_CSP_CORE_ENUMBITSET_H
#define _IN_CSP_CORE_ENUMBITSET_H

#include <csp/core/Platform.h>

#include <stddef.h>
#include <stdint.h>
#include <initializer_list>
Expand Down
1 change: 1 addition & 0 deletions cpp/csp/core/Generator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef _IN_CSP_CORE_GENERATOR_H
#define _IN_CSP_CORE_GENERATOR_H

#include <csp/core/Platform.h>
#include <memory>

namespace csp
Expand Down
31 changes: 14 additions & 17 deletions cpp/csp/core/Platform.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,19 @@
#include <stdint.h>
#include <time.h>

// cmake autogenerated export files
#include <csp/core/cspcore_export.h>
#include <csp/engine/cspengine_export.h>
#include <csp/engine/csptypes_export.h>

#include <csp/python/cspimpl_export.h>
#include <csp/python/csptypesimpl_export.h>

#include <csp/python/adapters/cspkafkaadapterimpl_export.h>
#include <csp/python/adapters/cspparquetadapterimpl_export.h>
#include <csp/python/adapters/cspwebsocketadapterimpl_export.h>


//TODO move Likely.h defines into Platform.h
#ifdef WIN32
#define NOMINMAX
Expand All @@ -14,20 +27,7 @@
#undef ERROR
#undef GetMessage

#define CSP_LOCAL
#define CSP_PUBLIC __declspec(dllexport)

#ifdef CSPTYPESIMPL_EXPORTS
#define CSPTYPESIMPL_EXPORT __declspec(dllexport)
#else
#define CSPTYPESIMPL_EXPORT __declspec(dllimport)
#endif

#ifdef CSPIMPL_EXPORTS
#define CSPIMPL_EXPORT __declspec(dllexport)
#else
#define CSPIMPL_EXPORT __declspec(dllimport)
#endif
#define CSP_PUBLIC __declspec(dllexport) // just for exceptions!

#define START_PACKED __pragma( pack(push, 1) )
#define END_PACKED __pragma( pack(pop))
Expand Down Expand Up @@ -90,10 +90,7 @@ inline uint8_t ffs(uint64_t n)
}

#else
#define CSPIMPL_EXPORT __attribute__ ((visibility ("default")))
#define CSPTYPESIMPL_EXPORT __attribute__ ((visibility ("default")))

#define CSP_LOCAL __attribute__ ((visibility ("hidden")))
#define CSP_PUBLIC __attribute__ ((visibility ("default")))

#define START_PACKED
Expand Down
7 changes: 5 additions & 2 deletions cpp/csp/core/QueueWaiter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
#ifndef _IN_CSP_CORE_QUEUEBLOCKINGWAIT_H
#define _IN_CSP_CORE_QUEUEBLOCKINGWAIT_H

#include <csp/core/Platform.h>
#include <csp/core/System.h>
#include <csp/core/Time.h>

#include <mutex>
#include <condition_variable>
#include <csp/core/Time.h>
#include <csp/core/System.h>


namespace csp
{
Expand Down
1 change: 1 addition & 0 deletions cpp/csp/core/TaggedPointerUnion.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef _IN_CSP_CORE_TAGGEDPOINTERUNION_H
#define _IN_CSP_CORE_TAGGEDPOINTERUNION_H

#include <csp/core/Platform.h>
#include <csp/core/System.h>

namespace csp
Expand Down
2 changes: 1 addition & 1 deletion cpp/csp/core/Time.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,7 +446,7 @@ inline std::ostream & operator <<( std::ostream &os, const Time & t )

// Time is internally stored as an int64_t nanoseconds since 1970.
// All DateTime objects are stored as UTC and should be treated as such
class DateTime
class CSPCORE_EXPORT DateTime
{
public:
DateTime() : DateTime( DateTime::NONE() ) {}
Expand Down
5 changes: 3 additions & 2 deletions cpp/csp/engine/AdapterManager.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef _IN_CSP_ADAPTER_MANAGER_H
#define _IN_CSP_ADAPTER_MANAGER_H

#include <csp/core/Platform.h>
#include <csp/core/Time.h>
#include <csp/engine/InputAdapter.h>
#include <csp/engine/PushInputAdapter.h>
Expand All @@ -16,7 +17,7 @@ class AdapterManager;

class Engine;

class ManagedSimInputAdapter : public InputAdapter
class CSPENGINE_EXPORT ManagedSimInputAdapter : public InputAdapter
{
public:
ManagedSimInputAdapter( csp::Engine *engine, const CspTypePtr &type, AdapterManager *manager, PushMode pushMode );
Expand Down Expand Up @@ -93,7 +94,7 @@ bool ManagedSimInputAdapter::pushNullTick()
return true;
}

class CSP_PUBLIC AdapterManager : public EngineOwned
class CSPENGINE_EXPORT AdapterManager : public EngineOwned
{
public:
AdapterManager( csp::Engine * );
Expand Down
1 change: 1 addition & 0 deletions cpp/csp/engine/AlarmInputAdapter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef _IN_CSP_ENGINE_ALARMINPUTADAPTER_H
#define _IN_CSP_ENGINE_ALARMINPUTADAPTER_H

#include <csp/core/Platform.h>
#include <csp/engine/InputAdapter.h>
#include <unordered_set>

Expand Down
9 changes: 5 additions & 4 deletions cpp/csp/engine/BasketInfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
#define _IN_CSP_ENGINE_BASKETINFO_H

#include <csp/core/Exception.h>
#include <csp/core/Platform.h>
#include <csp/engine/InputId.h>
#include <csp/engine/RootEngine.h>
#include <csp/engine/TimeSeriesProvider.h>
Expand All @@ -12,7 +13,7 @@ namespace csp

class Node;

class InputBasketInfo
class CSPENGINE_EXPORT InputBasketInfo
{
using TickedInputs = std::vector<INOUT_ELEMID_TYPE>;

Expand Down Expand Up @@ -188,7 +189,7 @@ class InputBasketInfo
bool m_isDynamic;
};

class DynamicInputBasketInfo : public InputBasketInfo
class CSPENGINE_EXPORT DynamicInputBasketInfo : public InputBasketInfo
{
public:
using ChangeCallback = std::function<void(const DialectGenericType & key,bool added, int64_t elemId, int64_t replaceId )>;
Expand Down Expand Up @@ -227,7 +228,7 @@ class DynamicInputBasketInfo : public InputBasketInfo
TimeDelta m_timeWindowPolicy;
};

class OutputBasketInfo
class CSPENGINE_EXPORT OutputBasketInfo
{
public:
OutputBasketInfo( CspTypePtr & type, Node * node, size_t size, bool isDynamic = false );
Expand All @@ -254,7 +255,7 @@ class OutputBasketInfo
bool m_isDynamic;
};

class DynamicOutputBasketInfo : public OutputBasketInfo
class CSPENGINE_EXPORT DynamicOutputBasketInfo : public OutputBasketInfo
{
public:
DynamicOutputBasketInfo( CspTypePtr & type, Node * node );
Expand Down
21 changes: 16 additions & 5 deletions cpp/csp/engine/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
include(GenerateExportHeader)
csp_autogen( csp.impl.types.autogen_types autogen_types ENGINE_AUTOGEN_HEADER ENGINE_AUTOGEN_SOURCE )

set(CSP_TYPES_PUBLIC_HEADERS
Expand All @@ -12,10 +13,7 @@ set(CSP_TYPES_PUBLIC_HEADERS
set(CSP_TYPES_SOURCE_FILES
CspEnum.cpp
CspType.cpp
DialectGenericType.h
PartialSwitchCspType.h
Struct.cpp
TypeCast.h
)

set(ENGINE_PUBLIC_HEADERS
Expand Down Expand Up @@ -83,10 +81,19 @@ set(ENGINE_SOURCE_FILES
${ENGINE_PUBLIC_HEADERS}
)

add_library(csp_types STATIC ${CSP_TYPES_SOURCE_FILES})
add_library(csp_types SHARED ${CSP_TYPES_SOURCE_FILES})
generate_export_header(csp_types
BASE_NAME csptypes
EXPORT_MACRO_NAME CSPTYPES_EXPORT
)
set_target_properties(csp_types PROPERTIES PUBLIC_HEADER "${CSP_TYPES_PUBLIC_HEADERS}")
target_link_libraries(csp_types csp_core)

add_library(csp_engine STATIC ${ENGINE_SOURCE_FILES})
add_library(csp_engine SHARED ${ENGINE_SOURCE_FILES})
generate_export_header(csp_engine
BASE_NAME cspengine
EXPORT_MACRO_NAME CSPENGINE_EXPORT
)
set_target_properties(csp_engine PROPERTIES PUBLIC_HEADER "${ENGINE_PUBLIC_HEADERS}")
target_link_libraries(csp_engine csp_core csp_types)

Expand All @@ -97,3 +104,7 @@ install(TARGETS csp_types csp_engine
RUNTIME DESTINATION ${CSP_RUNTIME_INSTALL_SUBDIR}
LIBRARY DESTINATION lib/
)

install(FILES ${CMAKE_CURRENT_BINARY_DIR}/cspengine_export.h
${CMAKE_CURRENT_BINARY_DIR}/csptypes_export.h
DESTINATION include/csp/engine)
3 changes: 2 additions & 1 deletion cpp/csp/engine/Consumer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
#ifndef _IN_CSP_ENGINE_CONSUMER_H
#define _IN_CSP_ENGINE_CONSUMER_H

#include <csp/core/Platform.h>
#include <csp/core/TaggedPointerUnion.h>
#include <csp/engine/BasketInfo.h>
#include <csp/engine/InputId.h>
Expand All @@ -13,7 +14,7 @@ namespace csp
class InputAdapter;

//Base of either regular Nodes or output adapters
class Consumer
class CSPENGINE_EXPORT Consumer
{
public:
Consumer( Engine * );
Expand Down
Loading
Loading