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

Draft: C workload bindings #11288

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Conversation

Wonshtrum
Copy link

@Wonshtrum Wonshtrum commented Apr 9, 2024

I'm utilizing the ExternalWorkload to test custom layers within the FoundationDB simulation. The layers under test are authored in Rust and are based on the foundationdb-rs project. The workload API is defined by the C++ header file ClientWorkload.h. Interoperability between Rust and C++ presents challenges, although creating a C++ to C shim was relatively straightforward. My main concern is the unstable ABI of the C++ API, as it relies on the compiler, linker, and system libraries. This proved to be an issue when transitioning from version 7.1 to 7.3, where the workload I was using suddenly broke due to a change in the representation of the std::string class as the fdbserver switched from gcc to clang, as detailed in this forum thread and issue #11298.

To address this, I propose that alongside the existing C++ API (utilized by JavaWorkload), pure C bindings should be made available to ease integration with other languages and mitigate the impact of changes in compilation environments. This aligns well with the location of ClientWorkload.h in the bindings/c folder.

I currently lack the necessary toolchain to compile the project, so it remains untested and likely doesn't compile. This PR is submitted to gather feedback on the proposed solution. Given my limited familiarity with the FoundationDB codebase, I resorted to various wrappers to minimize code changes. However, a more efficient approach might involve rewriting the ExternalWorkload or creating a dedicated C version.

In this draft, I've divided ClientWorkload.h into CWorkload.h and CppWorkload.h. Importing CppWorkload.h should mirror the behavior of the original ClientWorkload.h, ensuring compatibility with existing workloads utilizing this API. The C bindings introduce BridgeToClient and BridgeToServer structs that encapsulate collections of function pointers. If the "useCAPI" option is enabled in the ExternalWorkload configuration file, it will load the workloadInstantiate symbol (instead of workloadFactory) and invoke it with the server-bridge (allowing the C workload to interact with the C++ FDBWorkloadContext and GenericPromise<bool> classes). The returned client-bridge is then encapsulated within a C++ "translator" workload and assigned as the workloadImpl for the ExternalWorkload.

@Wonshtrum Wonshtrum force-pushed the c_workload_api branch 6 times, most recently from af050fd to d6a8fa4 Compare April 11, 2024 06:33
Signed-off-by: Eloi DEMOLIS <eloi.demolis@clever-cloud.com>
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.

None yet

1 participant