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

C++20 module support #1317

Open
Gaspard-- opened this issue Aug 20, 2024 · 8 comments · May be fixed by #1364
Open

C++20 module support #1317

Gaspard-- opened this issue Aug 20, 2024 · 8 comments · May be fixed by #1364
Labels
enhancement New feature or request

Comments

@Gaspard--
Copy link
Contributor

Describe the problem you are trying to solve.

flecs.h isn't small, and can slow down compilation of a file about 1-2 seconds on my machine.
This can add up over multiple files.
C++ modules allow to reduce this by only exposing relevant symbols and pre-compiling the module so that it can be reused.
I tried wrapping flecs.h into a module manually, but it seems due to how new is used within the library this does not work.
Maybe I'm missing something though.

Describe the solution you'd like

Add C++20 module support directly into flecs, allowing something like import flecs;.

From what I checked in flux, C++ module support boils down to adding a define (something like FLECS_EXPORT) which expands to export or nothing depending on another define.
Then, a module file is provided by the library which exports a module, enables this define and then includes the header to export the symbols.

@Gaspard-- Gaspard-- added the enhancement New feature or request label Aug 20, 2024
@Gaspard--
Copy link
Contributor Author

After looking at flecs.h some more, I feel like it may be possible to do something with the FLECS_API define, which seems to be used when flecs_STATIC is defined to mark symbols to export. Perhaps another define would be acceptable?

@Gaspard--
Copy link
Contributor Author

I tried a couple of things and:

  • Using the FLECS_API define would also require splitting out includes
  • Re-exporting symbols manually runs into problems with static symbols, Ex:
flecs-reexport.cpp:18:18: error: using declaration referring to 'Phase' with internal linkage cannot be exported
   18 |   using ::flecs::Phase;
      |                  ^
[...]/include/flecs/private/../addons/cpp/c_types.hpp:86:30: note: target of using declaration
   86 | static const flecs::entity_t Phase = EcsPhase;
      |               

Othewise, this simple file does actually work for quite a few usage cases:

module;

#define FLECS_CPP_ENUM_REFLECTION_SUPPORT 0
#include "flecs.h"

export module flecs;

export
namespace flecs
{
  using ::flecs::world;
  using ::flecs::entity;
  using ::flecs::component;
  [... other symbols etc.]
}

FLECS_CPP_ENUM_REFLECTION_SUPPORT seems to break things but i don't quite understand why yet (seems the compiler doesn't see one of the definitions of enum_init).

@Gaspard--
Copy link
Contributor Author

Gaspard-- commented Sep 16, 2024

It seems possible to work around the issue a little by using 2 modules:

module;

#define FLECS_CPP_ENUM_REFLECTION_SUPPORT 0
#include "flecs.h"

export module flecs_reexport;

export
namespace flecs
{
  using ::flecs::world;
  using ::flecs::entity;
  using ::flecs::component;
  [...]

  // Export C symbols
  using ::EcsPhase;
  using ::EcsDependsOn;
}

And then reexport the module in another module, in which we redefine the static symbols to make them accessible:

export module flecs;

export import flecs_reexport;

export
namespace flecs
{
  const flecs::entity_t Phase = EcsPhase;
  const flecs::entity_t DependsOn = EcsDependsOn;
}

This is of course far too hacky to be used within the library, but if any one else wants flecs in a module, this seems to be usable workaround in the meantime to be able to import flecs instead of #include "flecs.h".

@SanderMertens
Copy link
Owner

That does seem a bit hacky. Does this mean that the module'd version of Flecs won't be able to use enum reflection?

What other benefits do modules provide besides being able to do import flecs?

@Gaspard--
Copy link
Contributor Author

That does seem a bit hacky. Does this mean that the module'd version of Flecs won't be able to use enum reflection?

With this version yes, I haven't really dug into the issue to know what's the reason behind it though. It's possible a simple change could fix this. There's also always the possibility of a compiler bug since modules aren't widespread yet.

I think that a cleaner solution might also avoid this problem. I have to admit I'm still learning about what the proper ways to use modules are., but I suspect the proper approach is probably something along the lines of using a define to not use includes in flecs.h and include them separately in the module declaration. This seems to be how it's done in flux: https://github.com/tcbrindle/flux/blob/bf97ba7a3e9b5dc89b331d24d4c6dd82c1bd18af/module/flux.cpp

What other benefits do modules provide besides being able to do import flecs?

Nothing else I guess. Using modules does mean that no symbols except the exported ones are visible (so for example, we wouldn't expose the _ namespace, and we could also choose not to expose the C API) and does speed up compilation a bit for the consuming code.

Anecdotally I went from around 4 seconds to compile a file including flecs to 2.5 seconds when importing flecs, but my machine is particularly old.

@Gaspard--
Copy link
Contributor Author

I didn't think about the fact that maybe flux is header only, which makes it possible for them to define everything in modules directly when needed.
Module symbols and global fragment symbols (the default) aren't the same, so unless all the flecs C++ code is header only, the only way to add module support to flecs is by exporting using declarations, which is the approach I have tested locally (in the hacky way above).
It's not ready for a PR yet, but I might do one later if you think this is something you'd like to include in the repository. I'd probably want to do a few modifications to avoid the hacks and stuff.

Maintenance would amount to adding new types and functions when they are added to the C++ API. I don't know the exact capabilities of bake, maybe it's possible to generate it automatically with some kind of build script?

@SanderMertens
Copy link
Owner

Maintenance would amount to adding new types and functions when they are added to the C++ API.

That is unfortunately a non-starter, since it'd be way too error prone and difficult to test.

If it's something that can be done with just #define or a different way of organizing header files that'd be something worth investigating, but if it involves duplicating large amounts of code that make life of future contributors difficult, it's not feasible- especially if the only benefit is being able to do import flecs instead of #include "flecs.h" :)

@Gaspard--
Copy link
Contributor Author

Noted - I'll try to think for another solution, but for now it doesn't seem to be possible to tackle this issue easily :/

@Gaspard-- Gaspard-- linked a pull request Sep 19, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants