triton-inference-server/server

CMake problems for client library in v2.11.0

kpedro88 opened this issue · 4 comments

Description
This issue supersedes #2160, catching up to the latest release with the ongoing refactoring of different components into different repositories. As in the previous issue, this one relates to building just the client library (specifically the grpc client).

A few things have improved since #2160:

  • curl dependence is properly isolated to the http client
  • there are separate CMake flags to enable C and python clients, and they function correctly

However, some of the problems noted before still remain, and a few new ones have arisen:

  1. The top-level CMakeLists.txt in the client repo builds all third-party dependencies from source, so the the CMakeLists.txt in client/src/c++ must be used directly to link to pre-existing third-party dependency builds.
  2. The C++ client build tries to install all the libraries and headers from the third-party dependencies, so if the above path is taken, the libraries aren't available at the paths where CMake expects them.
  3. RapidJSON dependence should be isolated to the http client case, now in both the client and common repos. (In fact, the CMake setup links the json-related libraries to the grpc client libraries, even though they're never used.)
  4. The core repo is fetched for the CC_GRPC or PERF_ANALYZER cases, but it is only needed for the latter.
  5. It would be useful to have a CMake flag to handle the case in #2404.
  6. The protobuf part of the common repo build depends on python, even if the python client is not being installed.
  7. #2759 has not been addressed.

I think most of these are easier to address post-refactor than they were in the monorepo setup.

Triton Information
v2.11.0, built myself.

To Reproduce
See https://github.com/cms-sw/cmsdist/blob/IB/CMSSW_12_0_X/master/triton-inference-client.spec for the operations that I'm currently using to work around these various problems.

Expected behavior
More fine-grained control over the build and dependencies, using CMake flags where possible.

I can try to address at least the simple items myself, but I would like to understand the status of and plans for the ongoing refactoring, in order to avoid duplicated or conflicting effort.

@kpedro88 Thanks for listing all the possible enhancements. I agree we still don't have a really isolated build setup for client repo. We did plan to separate out perf_analyzer into its own repo but I don't believe that will be happening very soon.
Your contribution will be really appreciated. I am marking this as an enhancement but it might take us some time to get to this cleanup.

FWIW, I think most of these are bugs not enhancements.

If I don't enable/include Python, it's a bug to need or have to build python.
If I don't use HTTP, it's a bug to need or have to link with rapidjson.

For what it's worth, the client build doesn't even work if trying to just build the C API:

cmake -DCMAKE_INSTALL_PREFIX="$PWD/install" -DTRITON_ENABLE_CC_HTTP=OFF -DTRITON_ENABLE_CC_GRPC=OFF -DTRITON_ENABLE_PERF_ANALYZER=OFF -DTRITON_ENABLE_PYTHON_HTTP=OFF -DTRITON_ENABLE_PYTHON_GRPC=OFF -DTRITON_ENABLE_GPU=ON -DTRITON_ENABLE_EXAMPLES=ON -DTRITON_ENABLE_TESTS=OFF ..
make -j4
...
-- Up-to-date: /opt/src/triton/2.12.0/client/build/third-party/aws-sdk-cpp/lib/cmake/aws-cpp-sdk-core/aws-cpp-sdk-core-config.cmake
-- Up-to-date: /opt/src/triton/2.12.0/client/build/third-party/aws-sdk-cpp/lib/cmake/aws-cpp-sdk-core/aws-cpp-sdk-core-config-version.cmake
[ 28%] Completed 'aws-sdk-cpp'
[ 29%] Built target aws-sdk-cpp
Makefile:102: recipe for target 'all' failed
make: *** [all] Error 2

(This is arguably its own bug, but it goes directly to the overall concern in this issue, which is: "separability isn't actually designed in or tested.")

@jwatte The reason you see this issue is because you are running make -j4 without specifying the targets. Can you try make -j4 cc-clients ? Refer to our Dockerfile.sdk.

Closing issue due to lack of activity. Please re-open the issue if you would like to follow up with this issue.