Closed ilumsden closed 1 year ago
@JaeseungYeom I've fixed most of the comments you've made. The remaining things that are not yet addressed are:
__restrict__
__restrict__
(compiler extension) or restrict
(C99 keyword)clang-format
once everything is workingdyad_status_t
would be better? If you're curious what I'm specifically looking at, the relevant UCX code is hereconst
inline
keyword) when PFA is not enabled.I've resolved all the comments above that I have either fixed/addressed. The only ones that are still open are ones that either haven't been addressed or ones that I have responded to.
@JaeseungYeom this PR is almost done. The only comment you made above that I haven't yet resolved is the comment about renaming the error/return codes and the associated type. Otherwise, everything you've mentioned above has been resolved. I have also tested this PR using the ECP demo workflow, and it is working as intended.
At this point, I am going to open the PR as "ready for review". We can discuss the return code naming later this week. Then, all that will be left is any final reviewing you want to do.
Previously, each API re-implements the main (or "core") functionality of DYAD, such as Flux KVS put/get operations and Flux RPC calls. This PR abstracts all this shared functionality into an underlying
libdyad_core.so
library with the following API:The library also provides two convenience functions for producing and consuming data:
Besides creating this library, this PR updates the C "sync" API and the C++ "stream" API to use the new "core" library.
Currently, to use the "core" library, the Automake files do the following:
libdyad_core.la
using libtoolRPATH
of the API libraries to the path stored in the Autoconf variablelibdir
libdyad_core.so
intolibdir
This should mean that user code can use the API libraries without needing to manually link against
libdyad_core.so
. Alternatively,libdyad_core.la
can be made anoinst
target. However, this would prevent certain future decisions in the API libraries, such as making the C++ API a header-only library.Finally, this PR makes some minor reorganization changes to the repo to make it easier for new developers or people outside the project to understand where code is. More specifically, it makes the following changes:
common
toutils
lib
tostream
urpc
directory