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

Speed up CI jobs with parallelism and caching #10

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

Conversation

gregmedd
Copy link
Contributor

@gregmedd gregmedd commented Apr 13, 2024

Iterating on fixes for CI jobs can be time consuming, particularly when the issue exists at the end of a pipeline where some components take several minutes to build.

must go faster

I used these CI changes during the investigations leading to the CI fix in #9. By caching results for build dependencies, test cycles were reduced by around four minutes. The main time savings come from:

  1. Running stages with no interdependencies in parallel (up-cpp and zenoh-c)
  2. Caching artifacts for all stages leading up to the build for up-zenoh-example-cpp

The caches are tied to each repo's HEAD position, so if new commits are added the caches for that build stage are invalidated. Subsequent build stages will also re-run. Configuration parameters are provided for selecting a different named revision, if needed.

This uses the same pattern as eclipse-uprotocol/up-transport-zenoh-cpp#33, with only a small adjustment to caching structure and an additional stage for the examples from this repo.


This PR must wait until #9 is merged before it can be committed.
Use ➡️ this link ⬅️ to review the changes that are added by this PR.

MelamudMichael and others added 5 commits April 12, 2024 18:04
* aligning

* adding full build support

* fixiing compilation

* Update ci.yml

* Update conanfile.py

* Update ci.yml (x11)

* Update main_rpc_client.cpp

* Update main_sub.cpp

* Update ci.yml (x6)

* Update conanfile.py

* Update ci.yml

* Update conanfile.py (x2)

* Update CMakeLists.txt (x2)

* Update ci.yml

* Update CMakeLists.txt (x5)

* Update conanfile.py

* Update CMakeLists.txt (x3)

* Update ci.yml (x2)

* Update CMakeLists.txt (x2)

* Update ci.yml

* cleaning

* Update ci.yml

* Update CMakeLists.txt

* Update ci.yml

* Update CMakeLists.txt (x3)

* compilation fix

* part eclipse-uprotocol#1

* continuing

* continuing refactoring

* fixing

* aligning to changes

* aligning tests

* cleanup

* aligning

* Update CMakeLists.txt

* Update ci.yml

* Update conanfile.py

* examples: Updated to use upZenohClient

* examples: Use shared_ptr for reference counting

* Use a shared_ptr for reference counting and remove the explicit calls to
init/term.

* fixing compilation

* fixing samples

* cleanup

Co-authored-by: Bill Pittman <[email protected]>
Update CMakeLists.txt (x3)

Update ci.yml (x19)
Address failing CI build (missing zenoh-c symbols) by switching to
no-as-needed linking for zenoh-c and using a different library search
technique in cmake.

Adding -Wl,--no-as-needed is necessary when linking an application
against a static library where *another* library is dependent on that
static library's symbols. In this case, up-client-zenoh-cpp depends on
zenoh-c, but does not incorporate those symbols directly. At link time,
the linker will look at the symbols provided by zenoh-c and the symbols
needed for the application binary, determine none are needed, and
discard them. The -Wl,--no-as-needed flag tells the linker to embed
those symbols into the application binary anyway.

It was also necessary to explicitly pass a search directory and library
name to the linker instead of providing cmake a path to a static library
file. It is unclear why providing a library path was not working - the
documentation indicates it should. However, using standard -L<path> and
-l<library> flags did resolve the issue.

I tested with each of the above solutions independently and neither one
worked on its own. Both were required to fix the CI build.

This change also re-enables build for examples that were previously
disabled.
Builds on the pattern used in up-client-zenoh-cpp #33.
eclipse-uprotocol/up-transport-zenoh-cpp#33

Diagnosing CI issues can take forever, thanks to long build times for
dependencies that don't change very often. This change runs jobs in
parallel whenever possible, and caches results for prerequesites of the
example applications (based on hash of HEAD for their respective repos)

This can trim over 4 minutes out of CI build times when iterating on CI
changes.
Adds environment variable so that the target revision of dependency
repos can be set to something other than HEAD if needed. Helps prevent
churn / dependency failures with those repos are under active
development.

Note that the rev must be named, such as a branch or label. If it
wouldn't work with `git rev-parse`, then it won't work here.
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

2 participants