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

headers are too large #1325

Open
eladmaimoni opened this issue Jun 15, 2022 · 22 comments
Open

headers are too large #1325

eladmaimoni opened this issue Jun 15, 2022 · 22 comments

Comments

@eladmaimoni
Copy link

eladmaimoni commented Jun 15, 2022

I really like using vulkan-hpp. But I think the generated headers are way too large for most users.

This has significant productivity implications:

  • slows down compile time significantly, especially if not using precompiled headers.
  • IDE features such as auto complete tend not to work properly.

Another issue I encountered is that when I tried putting vulkan_raii.hpp as an include file in my precompiled headers, I got the weirdest compile errors about incompatible precompiled headers (this is probably an MSVC issue, but still - it took a long time to figure out that vulkan_raii.hpp is the issue).

I think there should be an option to generate "lighter" headers:

  • separating the "implementation" header needs to be included just once by a single .cpp file.
  • opting out of certain library features - for example the UniqueHandle api is probably unnecessary if one uses the raii API.
  • opting out of certain features. And I mean not even generating the code for them (just to be deactivated under a preprocessor command). Examples: VULKAN_HPP_NO_STRUCT_SETTERS, VULKAN_HPP_NO_STRUCT_CONSTRUCTORS VULKAN_HPP_HAS_SPACESHIP_OPERATOR etc.
@keryell
Copy link
Member

keryell commented Jun 18, 2022

Perhaps it is time to think about using C++20 modules in a near future.

@asuessenbach
Copy link
Contributor

@eladmaimoni

opting out of certain library features - for example the UniqueHandle api is probably unnecessary if one uses the raii API.

This is already possible by defining VULKAN_HPP_NO_SMART_HANDLE. If you see more candidates for features to be opted out, please let me know!

opting out of certain features. And I mean not even generating the code for them (just to be deactivated under a preprocessor command). Examples: VULKAN_HPP_NO_STRUCT_SETTERS, VULKAN_HPP_NO_STRUCT_CONSTRUCTORS VULKAN_HPP_HAS_SPACESHIP_OPERATOR etc.

I think, I don't get what you mean... more like those defines listed?
Or separate sets of headers where the corresponding code is not even contained?

Another issue I encountered is that when I tried putting vulkan_raii.hpp as an include file in my precompiled headers, I got the weirdest compile errors about incompatible precompiled headers (this is probably an MSVC issue, but still - it took a long time to figure out that vulkan_raii.hpp is the issue).

What compile errors did you get? Maybe I can do something to make life easier in such situations?

@eladmaimoni
Copy link
Author

@asuessenbach

Thanks for your comment,

This is already possible by defining VULKAN_HPP_NO_SMART_HANDLE. If you see more candidates for features to be opted out, please let me know!

Yes I found out about this flag after posting. So taking my words back. Regarding more features - I think that documenting the hpp generator code and allowing generation of thinner headers could be helpful.

I think, I don't get what you mean... more like those defines listed?
Or separate sets of headers where the corresponding code is not even contained?

I mean separate sets of headers where the corresponding code is not even contained. I think it could make IDEs and compiler much happier. Also, making the implementation code in separate headers / sections could also do good instead of having the implementations in the headers themselves. For instance, AMD memory allocator takes this approach.

What compile errors did you get? Maybe I can do something to make life easier in such situations?

The compiler errors I got emanated from a bug in the MSVC compiler / cmake. I was using precompiled headers that was shared (reused) among multiple cmake targets. When I included vulkan_raii, the generated precompiled header compiled just fine but got corrupted when copied by other targets that use it.

@asuessenbach
Copy link
Contributor

I think that documenting the hpp generator code ... could be helpful.

Totally agree.

allowing generation of thinner headers

You mean for those "power users" that generate their set of vulkan.hpp headers on their own, instead of just use what they get with the vulkan sdk? Might be a good approach. The minimal headers could be pretty small then!

making the implementation code in separate headers / sections

Is noted, and I will give that a try... might take a while, though.

@eladmaimoni
Copy link
Author

@asuessenbach Thank You!

@RYDB3RG
Copy link

RYDB3RG commented Jul 9, 2022

Would it be possible to add an option to the generator to create headers that contain only the declarations
and create cpp files that contain the implementations?
I think that removing the implementations from the headers would speedup IDE-tools a bit (and for release builds, header-only version could still be used)

@pc-john
Copy link

pc-john commented Jul 13, 2022

This thread really expresses my concerns. vulkan.hpp really slows down compilation by number of seconds for each compiled file where it is included. The problem became so big that I started to look for alternatives. Not sure what is slowing down the compiler (template magic, or something else). If there is some feature I am still looking for, it is the speed of compilation. All other new features makes me scared that the compilation will be even slower.

@theHamsta
Copy link
Contributor

Probably the easiest way to generate a custom trimmed version of the header is to create a custom vk.xml as this is used as the basis for the generator. One could filter the vk.xml to only include extensions used in the own project or only used struct+their dependencies. Filtering extensions with a allow+block list for the generator would maybe be a possibility for customization.

Maybe it would be useful to put extensions into separate headers?

Pairs of StructExtends alone takes 4924 lines of the header of the 14k vulkan.hpp header. They are only for additional compile time validation.

@theHamsta
Copy link
Contributor

Extensions could maybe also be gated by optionally gated by #ifdefs. So that with VULKAN_HPP_ONLY_OPT_IN_EXTENSIONS=1 only extensions with that have a VULKAN_HPP_OPT_IN_VK_KHR_copy_commands2 defined will be parsed by the compiler

@asuessenbach
Copy link
Contributor

I've conducted a couple of compile-time experiments now, and conclude a couple of things:

  • There seems to be no single compile-time killer feature that dominates or at least contributes a substantial part to the compile-time (other than that infamous reflection function (guarded by VULKAN_HPP_USE_REFLECT) and the spaceship operator (C++20, can be removed by VULKAN_HPP_NO_SPACESHIP_OPERATOR)).
  • Completely removing some functionality from the generated headers has a very marginal effect (if at all) compared to guarding that functionality by some define.
  • I have not checked if IDE features like auto completion get better with reduced headers.

That is, if someone would identify a feature that should be removable, that is should be guarded by some define, I'd be happy to add that.
And if someone gets substantial better compile times with reduced header sizes (maybe with other compilers than I have used) by not generating some features I can add options to do so as well.
Next thing to check on my end is the splitting of the headers into some interface and some implementation headers... but as said before, that might take a while...

@RYDB3RG
Copy link

RYDB3RG commented Jul 21, 2022

I hacked VulkanHppGenerator.cpp to remove all implementations from the generated headers but this has very little impact on cland-lsp :(

FYI: with clang -ftime-trace you can see where it spends most time:
image

@asuessenbach
Copy link
Contributor

@RYDB3RG Thanks a lot for your investigations!
Your results imply, that it' not a reasonable approach to separate the vulkan.hpp headers into a set of interface- and a set of implementation-headers.
I'm going to work on reducing compilation time next, and my first step was #1363. If you don't need/use any of the vk::to_string functions, you can define VULKAN_HPP_NO_TO_STRING to slightly reduce compilation time with vulkan.hpp.

@asuessenbach
Copy link
Contributor

As a side note: using vulkan.hpp (or vulkan_raii.hpp) as a precompiled header substantially reduces compile time.

@koplas
Copy link

koplas commented Feb 14, 2023

Are there any plans to support C++ modules? This would reduce compile times without having to use vulkan.hpp as a precompiled header. Visual Studio 2022 already supports modules and clang16 will be released with modules support.

A big limitation would be that selecting features with macros would no longer be possible and macros like VULKAN_HPP_DEFAULT_DISPATCH_LOADER_DYNAMIC_STORAGE have to be replaced.

@koplas
Copy link

koplas commented Feb 16, 2023

As an experiment, I modified VulkanHppGenerator.cpp to export C++ module files. Currently, I'm hitting Internal Compiler Errors with vulkan_raii, but the rest seems to work. I tried it in a sample project and as expected the overhead of importing modules is minimal.

My approach to making this project work with modules would be moving everything in vulkan.hpp to separate files, so this file only consists of includes and moving all macro definitions into one file, for example, moving the definitions in DispatchLoaderDefault.hpp into defines.hpp.

To try it out copy all files except vulkan_raii.ixx in https://github.com/koplas/Vulkan-Hpp/tree/modules-test/vulkan into your Visual Studio 2022 project with modules enabled and use it with import vulkan;. Note: This is an ugly hack and is not meant to be merged or used in production.

Nice, there is now a vulkan.cppm in this repo!

@swilson007
Copy link

Plus 1.

The version I'm using (which is a bit old compared to the repo) has a vulkan_structs.hpp that is 98k lines. Viewing that in my IDE is pain and suffering. One do-able in short-time solution here could just be to split this file into many as part of the generation (ex vulkan_structs_1.hpp, vulkan_structs_2.hpp, etc.) , and vulkan_structs.hpp could then include all the split files meaning no breakage of existing code.

I've actually made some design decisions around the size issue, which is to say that I've confined my use of vkhpp to only my core rendering library. The per-compile file time overhead of this header is just too much so any code or functionality I need outside of that library either uses my own wrappers or vulkan.h.

I don't expect the size issue to get fixed just due to the nature of the vkhpp, although I do like the idea of a custom vkhpp build for my project that eliminates features I don't use. I don't have time however to dig into the generation code, but if I had a generation executable with command line parameters to control the output I'd make time for that.

@pc-john
Copy link

pc-john commented Feb 21, 2023

I do like the idea of a custom vkhpp build for my project that eliminates features I don't use

Interesting idea. This might help to speed up compilation time.

One do-able in short-time solution here could just be to split this file into many as part of the generation (ex vulkan_structs_1.hpp, vulkan_structs_2.hpp, etc.)

Hm. Actually, I am just on opposite side :-) . My IDE is fast enough to show big files. So, splitting vulkan.hpp into subheaders or sub-subheaders just complicates the things to me. If I look for something particular in the headers, I have to think first which file is it located instead of just typing Ctrl-F and the keyword. I also need to keep number of files open instead of single one. So, for me personally, it is just annoyance to have vulkan.hpp split into multiple files. But I understand that others might have different preferences.

@swilson007
Copy link

Right @pc-john... the appeal of splitting the giant headers down depends on how we each use the header. For me, I mostly just hit my "go to definition" key over one of the structs so I can take a quick look at methods and parameters. I hop right to the struct in the vkhpp header, look around, and then I'm usually done. I very rarely scroll through the vkhpp headers. But for others, if there are scanning the header in a broader sense, then yes a split out header would be a pain.

I'll see how it goes for me in the future... If I get frustrated enough I may just take a few hours and write a script to split them out locally.

@swilson007
Copy link

@pc-john This is kind of funny... I just went looking for this GitHub project via google search of "GitHub vkhpp" and the first link was this: https://github.com/skyline-emu/vkhpp

"A custom version of Vulkan-Hpp with all the files split up so any single file won't go over the Android Studio file analysis size/length limit"

The project is defunct for years, just kind of funny that I found it unintentionally after having this discussion haha.

@pc-john
Copy link

pc-john commented Feb 22, 2023

Nice there is already solution!

In broader sense, it seems like having customizable generator that could generate customized headers for each project might be interesting idea. I know this one: https://github.com/Vulkan-FIT/vkcpp-gen. It includes GUI for selecting your options. You can skip unused extensions/generate core headers only, customize generated API and other things. This might reduce file size and compilation time. Not sure if this might be an idea for Vulkan-HPP as well (?).

@sharadhr
Copy link
Contributor

sharadhr commented Jan 9, 2024

Would either precompiled headers (MSVC, Clang) or, of course, vulkan.cppm be enough to close this?

@theHamsta
Copy link
Contributor

Would either precompiled headers (MSVC, Clang) or, of course, vulkan.cppm be enough to close this?

I know that pre-compiled headers provide an advantage for MSVC. But for clang/gcc they actually took longer than not using precompiled headers for Vulkan-hpp. Of course .cppm was even faster on clang than C-vulkan.h

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

9 participants