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

Optimize compilation time? #306

Open
rbx opened this issue Sep 8, 2020 · 3 comments
Open

Optimize compilation time? #306

rbx opened this issue Sep 8, 2020 · 3 comments
Milestone

Comments

@rbx
Copy link
Member

rbx commented Sep 8, 2020

@dennisklein My thoughts on this comment.

The question is if boost::signals can be hidden from public headers (or specifically FairMQDevice.h).

boost::signals propagates to FairMQDevice.h through:

  1. FairMQDevice.h --> ProgOptions.h --> EventManager.h --> boost::signals,
  2. FairMQDevice.h --> FairMQChannel.h --> Properties.h --> EventManager.h --> boost::signals.

For 1, ProgOptions can be made a forward declaration, but at at a price of forcing user to include ProgOptions.h wherever fConfig or GetConfig are used, which in majority of cases will be a small breaking change (due to missing include statements on user side).
For 2, I can actually remove Properties class entirely from FairMQChannel, it will only involve small amount of refactoring.

Ideally we would hide boost::signals from EventManager, but I don't see how it can be done, because the template types it takes propagate through to the user interface. Same thing goes for hiding EventManager in ProgOptions/Properties.

What do you think?

We also should consider the trade-off between compile time and potential runtime optimizations compiler can make. For config code this is probably not critical, but we don't have precise measurements for this atm.

@ktf
Copy link
Contributor

ktf commented Sep 9, 2020

Regarding having to include ProgOptions.h, I am completely fine with this because in any case this is hidden inside DPL in our case.

@dennisklein
Copy link
Member

Let's think how we can get rid of boost::signals in EventManager. If we switch to C++17 now anyways I am sure we have enough tools to re-implement it much more lightweight.

@ktf
Copy link
Contributor

ktf commented Sep 9, 2020

I would actually be interested in any "lightweight" alternative for my ServiceRegistry as well.

@dennisklein dennisklein added this to the v1.6 milestone Aug 12, 2022
@dennisklein dennisklein modified the milestones: v1.6, next Jun 7, 2023
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

No branches or pull requests

3 participants