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

feat(autoware.repos): add fork of ament_cmake for faster build #4141

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

mitsudome-r
Copy link
Member

@mitsudome-r mitsudome-r commented Feb 1, 2024

Description

This PR will add the fork of ament_cmake which makes the build of Autoware faster. This originated from the discussion in https://github.com/orgs/autowarefoundation/discussions/3491. We were waiting for this PR to be merged to the upstream, but we already have issues with build check CI taking too much time in autoware.universe so I think it's worth using the fork until the PR is merged to upstream.

Related links

https://github.com/orgs/autowarefoundation/discussions/3491
ament/ament_cmake#448

Tests performed

run vcs import and check with colcon build.

Notes for reviewers

Interface changes

Effects on system behavior

On my machine, build time has changed from:

Summary: 340 packages finished [48min 18s]

to:

Summary: 363 packages finished [30min 19s]

Pre-review checklist for the PR author

The PR author must check the checkboxes below when creating the PR.

In-review checklist for the PR reviewers

The PR reviewers must check the checkboxes below before approval.

  • The PR follows the pull request guidelines.
  • The PR has been properly tested.
  • The PR has been reviewed by the code owners.

Post-review checklist for the PR author

The PR author must check the checkboxes below before merging.

  • There are no open discussions or they are tracked via tickets.
  • The PR is ready for merge.

After all checkboxes are checked, anyone who has write access can merge the PR.

@xmfcx
Copy link
Contributor

xmfcx commented Feb 13, 2024

On my machine, build time has changed
from:

Summary: 340 packages finished [48min 18s]
to:

Summary: 363 packages finished [30min 19s]

@mitsudome-r did you have ccache configured for these measurements?

If you did, the cached build artifacts in ~/.cache/ccache/ will affect these numbers.

@VRichardJP
Copy link
Contributor

VRichardJP commented Feb 13, 2024

@xmfcx https://github.com/VRichardJP/ament_cmake/tree/faster_deduplicate_humble

Currently on top of humble: ament/ament_cmake@humble...VRichardJP:ament_cmake:faster_deduplicate_humble

@xmfcx
Copy link
Contributor

xmfcx commented Feb 13, 2024

I've made multiple tests today, with and without ccache.

Specs:
OS: Ubuntu 22.04 jammy
Kernel: x86_64 Linux 6.5.0-15-generic
CPU: AMD Ryzen 9 5900X 12-Core @ 24x 3.7GHz
GPU: NVIDIA GeForce RTX 3090
RAM: 5962MiB / 64214MiB

Cuda compilation tools, release 12.3, V12.3.107

Scenario Simulator v2 was also compiled.

TVM packages were built too (I think we build them all the time now but I'm not sure).

Total package count: 392

Also in my tests, I've switched between the humble branch of ament_cmake and Vincent's branch that comes with this one.

So the we had to recompile ament_cmake in both cases.

Tests with CCache

First 4 tests were made with ccache, therefore the experiments are in chronological order since they all use the cache before them.

Test Number ament_cmake Version Build Time
1 New 10min 7s
2 Old 25min 49s
3 New 5min 34s
4 Old 23min 47s

Tests without CCache

In these tests, the order is not important since there is no cache.

Test Number ament_cmake Version Build Time
5 Old 52min 15s
6 New 34min 27s
7 Old 51min 3s
8 New 33min 53s

In tests 5 and 6 I was also light browsing on some tabs but on 7 and 8 PC was completely focused on compiling.

In the end, with the new version, planning simulator demo works as expected.

I've also rebased the 2 commits we had over latest humble branch to be in sync.

Let's merge this!

@xmfcx
Copy link
Contributor

xmfcx commented Feb 13, 2024

@VRichardJP it turns out we had a copy of your branch on autowarefoundation fork.

https://github.com/autowarefoundation/ament_cmake/commits/feat/faster_ament_libraries_deduplicate/

And it also has a commit from @TakaHoribe it seems.

I have rebased it over latest humble too, it should be alright!

@xmfcx
Copy link
Contributor

xmfcx commented Feb 13, 2024

Just to be sure this passes the tests, I will run build and test job on this manually before merging. (Normally it runs daily)

@xmfcx
Copy link
Contributor

xmfcx commented Feb 13, 2024

@mitsudome-r I've also duplicated your branch to autowarefoundation/autoware
to be able to run the CI on them.

build-main: https://github.com/autowarefoundation/autoware/actions/runs/7887742760

build-main-self-hosted: https://github.com/autowarefoundation/autoware/actions/runs/7887791883

@xmfcx
Copy link
Contributor

xmfcx commented Feb 13, 2024

Screenshot from 2024-02-13 17-39-05

https://github.com/autowarefoundation/autoware/actions/workflows/build-main.yaml

We have started getting no space left for main autoware branch too @mitsudome-r 😨

We should speed up reducing the container size Autoware's size?

@xmfcx xmfcx merged commit af0fbe3 into autowarefoundation:main Feb 13, 2024
16 of 19 checks passed
@xmfcx
Copy link
Contributor

xmfcx commented Feb 13, 2024

Merged regardless, these CI's were only building, not testing anyways.

oguzkaganozt pushed a commit that referenced this pull request Feb 19, 2024
@felixf4xu
Copy link

universe/external/ament_cmake: # TODO(mitsudome-r): remove when https://github.com/ament/ament_cmake/pull/448 is merged

and I see ament/ament_cmake#448 is already merged.

@mitsudome-r

@xmfcx
Copy link
Contributor

xmfcx commented May 11, 2024

@felixf4xu should be here: https://github.com/ament/ament_cmake/commits/humble/ (backported to humble) and also be available as part of the debian packages. (Which also takes up to a month after being backported)

But this package is very light, so it's alright to have it as a fork here.

@felixf4xu
Copy link

then should I uninstall the ros-humble-ament-cmake-core? I have a warning at the start of the colcon build, something like ament-cmake-core is already installed.

my apt search results like this:

ros-humble-ament-cmake-core/jammy,now 1.3.8-1jammy.20240217.022939 amd64 [installed,automatic]

@xmfcx
Copy link
Contributor

xmfcx commented May 11, 2024

you do not need to uninstall it, the warning probably can be ignored. If you share the exact logs, I can answer better.

@felixf4xu
Copy link

[1.980s] WARNING:colcon.colcon_core.package_selection:Some selected packages are already built in one or more underlay workspaces:
        'ament_cmake_core' is in: /opt/ros/humble
If a package in a merged underlay workspace is overridden and it installs headers, then all packages in the overlay must sort their include directories by workspace order. Failure to do so may result in build failures or undefined behavior at run time.
If the overridden package is used by another package in any underlay, then the overriding package in the overlay must be API and ABI compatible or undefined behavior at run time may occur.

If you understand the risks and want to override a package anyways, add the following to the command line:
        --allow-overriding ament_cmake_core

This may be promoted to an error in a future release of colcon-override-check.

@xmfcx
Copy link
Contributor

xmfcx commented May 11, 2024

Yes, you can safely ignore this. This warning is an issue when you want to interoperate between system and workspace libraries/messages. Bıt for these cmake packages, it won't be a problem. It will use the workspace packages.

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.

4 participants