deduplicate dependency lists of exported code generation targets #1123
Conversation
|
@pseyfert Please provide a minimal reproducible example which demonstrates this. I would like to first understand where it is coming from / why the entries are duplicated. |
|
thanks for the quick response. i'll let you know when i have something. |
|
https://github.com/pseyfert/catkin_generated_target_reproducer as you'll see I use
We didn't (yet?) check if we're seeing a side effect of that (mostly because we first tried to get a reproducer close to our setup w/o rewriting from catkin_simple to catkin w/o catkin_simple) |
|
Hello, we were finally able to reproduce the duplication with catkin only, thus without catkin simple. It seems the issue happens with diamond structured dependencies involving more than 2 packages. We have the following structure in the reproducer: The following print just a line above the change in this MR shows that exported targets are duplicated in the list. The duplication starts only at The reproducer without catkin simple can be found here: |
|
@tfoote @mjcarroll FYI. |
|
I have reproduced this behavior using the supplied repository: In the example, it manifests on in When applying this fix, it does remove duplicates. I'm still a bit unsure, though, if this is a regression in behavior, as that particular piece of code has not been changed in quite some time. @dirk-thomas was there a particular reason that the |
In case there is a misunderstanding. When I wrote
we have on our side reorganised our packages and dependencies and ran into the issue in the process. So it's not necessarily a catkin regression, might also be we never had a problematic dependency graph. |
I guess it just never was an issue before. Since the order of targets is important I doubt |
replace _list_append_unique by _list_append_deduplicate
works in our project. updated. |
|
Just as a follow up info; the present issue got painful for us in a workspace with 100+ packages and many inter-dependencies. Towards the end the cmake configuration step alone was consuming around 20 GB of RAM (!) and took several minutes for certain top-level packages. With the proposed fix the time and memory consumption of the cmake step went back to being insignificant. |
TL;DR
deduplicate the list of
_EXPORTED_TARGETS(from the comments I read this is about codegen targets).background
In one of our projects the cmake configure time recently increased significantly (10s → 2min) and memory consumption of cmake increased noticably.
With cmake's trace capabilities we found that in the
<ourproject>_EXPORTED_TARGETSlist took a lot of time to grow and by adding printout to the generated<workspace>/devel/share/<ourproject>/cmake/<ourproject>Config.cmakefile, noticed that there were many many duplicate additions ofIt seemed to us that deduplication of the list of exported targets is desirable and changed the template, just like the deduplication is already done with the list of library dirs in the line before.
Configure time went down to O(10s) again and build and tests seem fine.
second thoughts
I'm a bit suspicious though why deduplication isn't done already as the means to do so are there and even applied in the line before (and quickly glimpsing at the git history, _list_append_unique was already in that line when _EXPORTED_TARGETS got added).
For completeness sake, we're still have on our todo list to double check if we're seeing just a symptom of an underlying issue (potentially on our side) that causes these long duplications (I don't see a circular dependency in our stack). Also,we're using catkin_simple as catkin frontend which handles message generation targets for us.