fix: Fixed has target check taking ages for large projects

[jmachowinski]

The old implementation took up to 30 seconds for packages, were a lot of targets existed. This is a common case for interface packages containing a lot of messages.

[wjwwood]

After reading https://stackoverflow.com/questions/23849953/how-to-check-existence-of-the-target-in-a-makefile, I'm wondering if we shouldn't also use --dry-run and maybe use the strategy suggested there where we check the output of the data base with --print-data-base? I'm not certain it provides more benefits, but maybe it would help to avoid unintended (and in some cases expensive time-wise) side effects.

With that suggestion aside, there's just a few small technical questions and requests before this is lgtm.

[wjwwood]

I tested this on macos and I didn't notice or see any issues building the ros2.repos up to rclcpp and a little past that.

[jmachowinski]

I had some issues with this patch, it does not seem to work in special situations.
I currently don't have time to look into it, therefore I will convert this PR into a draft.

[cottsay]

What sort of behavior does this exhibit if the CMake generator is Visual Studio? Also, what if the system's language is not set to English - is make's output localized?

[jmachowinski]

What sort of behavior does this exhibit if the CMake generator is Visual Studio?

Unchanged

Also, what if the system's language is not set to English - is make's output localized?

AFAIK make is not localized, at least it is not on my german localized ubuntu

[jmachowinski]

After reading https://stackoverflow.com/questions/23849953/how-to-check-existence-of-the-target-in-a-makefile, I'm wondering if we shouldn't also use --dry-run and maybe use the strategy suggested there where we check the output of the data base with --print-data-base? I'm not certain it provides more benefits, but maybe it would help to avoid unintended (and in some cases expensive time-wise) side effects.

--dry-run generated a lot of not needed output, which is not done by the question mode. As I don't check on the return code, but parse the std_err instead, we should be fine

[jmachowinski]

This version is working, and tested. Additional tests would be appreciated though, as I only have a linux machine.

[cottsay]

Beyond the linting errors that CI identified, I am also concerned that this change is coupled to GNU Make. The -q flag has a different (and incompatible) meaning on BSD: https://man.freebsd.org/cgi/man.cgi?make(1)

That said, BSD isn't an explicitly supported platform, but it still doesn't feel good to break it just for an optimization. I'll do a little more poking around to see if I can see another solution.

[jmachowinski]

@cottsay ping

[cottsay]

It looks like the -n flag is available on all of the platforms I tested. It prints the command that WOULD be executed for a target, but importantly returns non-zero if the target doesn't exist.

It appears to be a lot faster than help in my tests. Would that work for you?

[jmachowinski]

I just testet -n vs -q , on our system -n seems to be slower but its still a major improvement over the current state :
With -q

Starting >>> cm_interfaces
Finished <<< cm_interfaces [2.87s]                     

Summary: 1 package finished [3.59s]

Starting >>> cm_interfaces
Finished <<< cm_interfaces [2.93s]                     

Summary: 1 package finished [3.67s]

with -q

Starting >>> cm_interfaces
Finished <<< cm_interfaces [4.61s]                     

Summary: 1 package finished [5.34s]

Starting >>> cm_interfaces
Finished <<< cm_interfaces [5.41s]                     

Summary: 1 package finished [6.14s]

Starting >>> cm_interfaces
Finished <<< cm_interfaces [4.56s]                     

Summary: 1 package finished [5.29s]

I still wonder, about the impact of the text output in colcon though...

time make -q install

real    0m0,343s
user    0m0,330s
sys     0m0,011s

time make -n install
real    0m0,960s
user    0m0,812s
sys     0m0,142s

The time spend in make increases only about 600ms, vs 1.6 seconds in colcon.

[cottsay]

Let's try a completely different approach: #144

This appears to be significantly faster even for medium-sized projects. Please give it a shot on your large project. You may need to pass --cmake-force-configure if you're not building a clean workspace to ensure the query file gets created.

[jmachowinski]

Starting >>> cm_interfaces
Finished <<< cm_interfaces [1.89s]                     

Summary: 1 package finished [2.45s]

#144 looks like a way better solution. I like it !

Closing in favor of #144

[cottsay]

Glad it worked well for you.

This actually happens twice for ROS projects, which is probably why the timing you see coming from colcon is so different from the raw make invocation timing.

To be honest, the bespoke code snippets for the individual generators have grown pretty long-in-tooth and we're performing FAR too many process invocations during each CMake build. Most if not all of the information we need to collect from those invocations is part of the codemodel file API, so there may be a small refactor in the near future to shave off some more time from CMake jobs like this.