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

Have ability to split the build in CI #7076

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Have ability to split the build in CI #7076

wants to merge 8 commits into from

Conversation

mc-nv
Copy link
Collaborator

@mc-nv mc-nv commented Apr 5, 2024

MOTIVATION:
Triton Server CI is monolithic, I would like to add ability splitting the CI and build it per backend if needed

CHANGES:

  1. Added flags --ci-split and --cache-from-map
  2. Update logic of file creation in case of flag --ci-split

VERIFICATION:

$ ./build.py --enable-all --split-build --dryrun
$ ls build
cmake_build                 cmake_build_backend_ensemble  cmake_build_backend_onnxruntime  cmake_build_backend_pytorch  cmake_build_backend_tensorflow  cmake_build_cache_redis  Dockerfile
cmake_build_agent_checksum  cmake_build_backend_fil       cmake_build_backend_openvino     cmake_build_backend_repeat   cmake_build_backend_tensorrt    cmake_build_collect      Dockerfile.buildbase
cmake_build_backend_dali    cmake_build_backend_identity  cmake_build_backend_python       cmake_build_backend_square   cmake_build_cache_local         docker_build             Dockerfile.cibase
$ ./build.py --enable-all --dryrun
$ ls build
cmake_build  docker_build  Dockerfile  Dockerfile.buildbase  Dockerfile.cibase

build.py Outdated Show resolved Hide resolved
build.py Outdated Show resolved Hide resolved
build.py Outdated Show resolved Hide resolved
mc-nv and others added 2 commits April 5, 2024 16:05
Co-authored-by: Ryan McCormick <rmccormick@nvidia.com>
Co-authored-by: Ryan McCormick <rmccormick@nvidia.com>
build.py Outdated Show resolved Hide resolved
@mc-nv mc-nv requested a review from rmccorm4 April 8, 2024 19:50
Copy link
Contributor

@nv-kmcgill53 nv-kmcgill53 left a comment

Choose a reason for hiding this comment

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

Just an API comment, since API's are sticky you try to get them right the first time because the overhead of changing them later is high.

build.py Outdated Show resolved Hide resolved
Comment on lines +2357 to +2367
parser.add_argument(
"--cache-from-map",
action="append",
required=False,
help="A set of docker images to `--cache-from` where applicable to help speedup builds",
default=[
"tritonserver_buildbase",
"tritonserver_buildbase_cache0",
"tritonserver_buildbase_cache1",
],
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I understand that it was called a map before, but it's really just a list. What do you think about changing this to something like --cache-image or --cache-images? We can make use of the multiple arguments in a couple of different ways. We can use the action="append" or the nargs='+' fields (SO comment for example). I might lean towards the --cache-image with action="append" API just because we have precedent.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I think name was used to keep consistency with original command which is relates to: docker build --cache-from

$ docker build --help | grep cache-from
      --cache-from stringArray        External cache sources (e.g., "user/app:cache", "type=local,src=path/to/dir")

Co-authored-by: Kyle McGill <101670481+nv-kmcgill53@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants