tesseract-robotics / tesseract

Motion Planning Environment
http://tesseract-docs.rtfd.io
Other
257 stars 89 forks source link

Leverage forward declarations to improve compile times #990

Closed Levi-Armstrong closed 5 months ago

Levi-Armstrong commented 6 months ago

This cuts the build time from 4 minutes and 30 seconds to 2 minutes and 45 seconds.

marip8 commented 6 months ago

Are the fwd.h headers not problematic by declaring a bunch of classes that don't necessarily get used in each source file? Why not just declare the specific classes that each header needs to reference?

Levi-Armstrong commented 6 months ago

Are the fwd.h headers not problematic by declaring a bunch of classes that don't necessarily get used in each source file?

It is not because you can declare things as many time as you want but you can only have one define it once.

Why not just declare the specific classes that each header needs to reference?

Because forward declaration can be problematic especially when using in template classes, so it is encouraged that developers provide this file to let users know what types are okay to use as forward declarations. An example of this is the #include <iosfwd> provided by the standard libraries to avoid having IO headers in your headers.

codecov[bot] commented 6 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 90.17%. Comparing base (d361f7d) to head (fccd356).

Additional details and impacted files [![Impacted file tree graph](https://app.codecov.io/gh/tesseract-robotics/tesseract/pull/990/graphs/tree.svg?width=650&height=150&src=pr&token=nh4aHZzgpR&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tesseract-robotics)](https://app.codecov.io/gh/tesseract-robotics/tesseract/pull/990?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tesseract-robotics) ```diff @@ Coverage Diff @@ ## master #990 +/- ## ========================================== - Coverage 90.91% 90.17% -0.74% ========================================== Files 280 280 Lines 15876 15701 -175 ========================================== - Hits 14434 14159 -275 - Misses 1442 1542 +100 ``` [see 181 files with indirect coverage changes](https://app.codecov.io/gh/tesseract-robotics/tesseract/pull/990/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=tesseract-robotics)
Levi-Armstrong commented 6 months ago

@johnwason Would you have time to look into what the issue is with the windows test failures?

johnwason commented 6 months ago

Why not just use precompiled headers instead of reworking everything?

Levi-Armstrong commented 6 months ago

Why not just use precompiled headers instead of reworking everything?

At this point I am almost finished updating everything.

johnwason commented 6 months ago

@Levi-Armstrong my fixes didn't work... go ahead and force push to revert my last two commits.

johnwason commented 6 months ago

I am not seeing the serialization unit test failures locally. I am seeing issues with the static loader plugin tests.

Levi-Armstrong commented 6 months ago

@johnwason This may be related.

johnwason commented 6 months ago

I am not seeing the serialization unit tests locally. I am investigating but so far don't have an explanation. Maybe try clearing the github action cache completely?

Levi-Armstrong commented 6 months ago

Can I manually clear the cache?

johnwason commented 6 months ago

You can see all the caches here: https://github.com/tesseract-robotics/tesseract/actions/caches

Not sure how to "clear all" though

johnwason commented 6 months ago

I updated to the newest vcpkg boost version and now I am seeing the test failures. I will keep trying to trace the error.

johnwason commented 6 months ago

Looking at the CI, the Conda Windows CI is working with Boost 1.82, while the vcpkg Windows build is failing with Boost 1.84. This may be a problem with Boost 1.84.

johnwason commented 6 months ago

Yeah I am seeing the same failures on the scheduled builds on master. This looks like a boost problem.

johnwason commented 6 months ago

I ran a matrix build of different boost versions, and only 1.84 is failing. https://github.com/johnwason/tesseract/actions/runs/8242279639/job/22540993793

I set the vcpkg revision to use boost 1.83. I recommend opening an issue with boost serialization.

Levi-Armstrong commented 6 months ago

@johnwason I believe I removed your change when I force pushed. What change do I need to make to pin the version?

johnwason commented 6 months ago

@Levi-Armstrong I pushed the commit

johnwason commented 6 months ago

See boost/serialization issue https://github.com/boostorg/serialization/issues/309