All right, so hello everyone. I'm a done. I work on system D and I spent about a
month or so last year cleaning up includes in system D. So I'm here to talk about that a
little and hopefully it might be useful for the people in the room here as well.
So let's start with why this matters. These are like some stats from after I did the work.
What it basically came down to was that I got like a 32% increase or decrease luckily in
a compilation times for a from scratch build and subCI jobs became faster by 50% just by removing
includes from system D source and had a files that were unused. So how did I get started
on this project? Well, I was just working on system D at some point and I use Clang D as my
LSB and VS code and it's just crowned to a halt. Seemingly randomly as long as I had the
project open for long enough and so without doing any like proper investigation I decided for
myself this must be because we include to many headers. So I did a bunch of work to fix all the
unused includes and then I figured out the issue was something totally different but well we still
clean up the includes right so mission achieved. And so I ended up I did a small I don't know
calculations not entirely correct but I ended up removing at least or at least yes more than
3000 includes from all of system D source files. And then the question is why how do I how did I go
about it? So you want to do two things you want to remove like unused includes and so this is mostly
about source files where you just want to remove every include that doesn't provide a symbol or
a definition or whatever that isn't being used but then you also want to remove or minimize the number
of transient includes and this is more about header files that are included by other header files. So
the way you do this usually is with forward declarations it's with moving macro implementation
details into the source file and by minimizing the amount of online functions. So forward declarations
mean it's easy or you don't have to include the full header file providing the symbol macro
implementation means that if you have like I have to like add that like we don't write
C++ right so we need macros for a lot of stuff so that's why we focus on macros a little bit as
well and so like macros are basically a full function and because they need to be the final
header file every include for to implement that macro needs to be an header file which pulls
in a bunch of stuff which slows down compilation and then the online functions we had quite a
few of those as well and depending on what they do they also need to pull in an extra header
which is ways full if you move it to the source file and rely on altiodity optimization you
can avoid the include and you again reduce the amount of the includes that are required to be included
by the source files. So the first thing I did was actually fixing all the circular inclusions
in this so we did this quite a bit across statements so the what would you would think of like
you had a header file it had a four bunch of four declarations for the structures was going to
declare later then the debuts of includes and then it actually defined the includes like this
was this was absolutely horrifying when you would notice it like nobody would write same C++
this way these T's days but we did it a bit all over the place so I basically got rid of all of
that and the the clank tidy check I used for this is missk header includes cycle so I think this one
is like absolutely amazing to be able to have every project and just get rid of all the circular
includes there's always a way to get rid of them and then I'll make my sense to have in the first place
so I would recommend everyone to implement that one and I have it enabled by default and I also had
a lot of issues with clank decrashing for seemingly unknown reasons and removing all the circular
fixed that so I'm not sure what bug there was there but that also helped with that as well
this was all like done by hand basically enabling the check running and then figuring out how
I could best fix the circular includes you need to think through your headers or your source
file and header file layout a little bit because you might need to move strokes around to be
to avoid circular includes in the first place so it can get gnarly but I think it's almost always
worth it to fix these and then we had to integrate clank tidy in the build system so we use
mess on which has integration and as it's clank tidy target see make has it a similar thing where
if you configure it it will just run clank tidy every time you compile the relevant source file
and so I tried using this but there are a bunch of issues it doesn't really you can't
add dependencies to the clank tidy target on sources that are generated by your build system
which means that it will fail to run on those it runs on all source files regardless of whether
their build or not and when you have a lot of conditional compilation and a project like
system either also brings issues and you can't configure the target so that didn't really
run great all of the m is a run clank tidy script I try that as well but it introduces its own form
of scheduling right it runs parallel jobs and everything itself but we don't need that we already
have mess on this build system it can run parallel jobs it can run tests and parallel and so that's
what we ended up doing in the end we instead of like running clank tidy as part of the build we
run it as a unit as and this has various benefits the main thing being that as soon as we get
into the actual unused headers depending on the build configuration you're going to have that
clank tidy check trigger more often or not and on different headers if you're building without
some dependency or anything different right you can all of those build system configurations can
end up giving you more unused headers so getting your entire build configurations like every
single combination of build system option like library enabled or not right this library might
be enabled that order library might be disabled or anything else can give you very
different results so if you enable this on build right if you do grunk clank tidy every time
you use a compiler source file it's just impossible to get working so I find it much easier
as a unit test so that you can put it in its own suite and only run it for very specific build
configurations the first thing I thought it was actually IWU so include what you use which is
a project by Google which uses the clank c++ API and then you immediately run into the issue
that clank c++ API does not offer backwards compatibility and so like immediately it was already
depending on features from clank in get main whereas my distribution was only shipping the
stable version it wouldn't even compile so that was kind of annoying it also had a bunch of
missing features like we use the clean up attributes from gcc and well clank as well in system
need to have the structures just like c++ that wasn't supported so for the clean up attribute
you pass it a function and that function was not detected so it would mark a bunch of stuff as
unused even though it was being used in the clean up attribute I send a PR for that I fixed some
other stuff as well but in the end I didn't end up using it because of the compatibility issues
with the clank c++ API and because the clank thing ended up being much nicer to use so the clank
thing is clank include clean so this was a project that originated in clank d the language server
but then they decided to pull it out into its own tool which is clank include clean it's less
opinionated right what I will you I will you I will do is it will report like it will remove
an used includes but it will also always add missing includes so it insists on directly including
the header providing a symbol for everything a source file so it doesn't like if you have transitive
includes it will still add all the transient sorry transitively included includes into the
source file so you end up with huge changes which I mean it's arguable whether that model is
useful enough but I wanted to keep the changes somewhat minimal and clank included it easily
allowed me to only focus on removing unused includes and keeping the transient includes in place
as they are except with all VM which just makes it much easier to use interestingly enough
and that exactly the same bug as I will you or you which is the cleaner attribute also wasn't
passed fixing it in clank include cleaner was a bit more hairy than fixing it in include what you
use but I ended up figuring it out eventually I will include what use was also much easier to compile
in LVM or I had to spend hours waiting for it to build it finish but eventually I figured it out
and then the the best thing is that it is integrated into clank tidy so instead of having to
figure out the built integration for that in system D right like or invoking a separate tool
by the built system after thing no I just had to enable a single check in clank tidy and I got
all my unused includes reported file unit as failures so then you get to how to manage the source
code to make this somewhat like viable like I said one of the solutions to solving or to removing
transient includes in header files is to use forward declarations instead but repeating the same
forward declarations in every single file does not scale like it ends up being incredibly for posts
so the solution we found there was simply like forward declaration errors I mean we didn't find
the solution I think the standard library even does is itself so we ended up adopting that as well
we simply have like a few files like we call them forward or age and with a prefix and that's
just simply all the forward declarations we can imagine right so all the structs all the even all
the enums reformer forward declare though we can forward declare their size yet I think that's
coming in C 2020 or 23 somewhere once we can do that we can get rid of even more headers
because we can forward declare the size and when we want to have to include the header file declaring
the enum anymore and then to make things easy to use we also made those forward dot h files include
the most commonly included header files for C and so like standard n dot h types dot h like there's a
bunch of stuff error node dot h which you are always going to end up using so it doesn't make
sense to repeat the include for every single source file you just put it in one of those forward
dot h letters and you get it for free by just including that single file and you're not compiling
more than you are parsing more than you did before because it was going to get include files
on trends and include anyway um so then there is a conditional compilation like I said like
depending on what you're enable the uh the uh just includes are going to be different and so instead
of trying to tackle that problem and making sure every single build configuration works
what I ended up doing is to just define a very specific build configuration in which we run
clank tidy and that's the one where we that that's the one that runs the CI and the one that we
that we like officially support and that configuration is just the one where we enable every single
feature we have so that we get the most coverage for all the different libraries and stuff that we
enable of course this doesn't work when it's enabled or not but if you have to make a choice
between one library or the order which we do have in a few cases and you have to make a choice
and for those cases I did end up making sure that both configurations work so that means
like putting a bunch of includes behind if that's to make sure they're not unused when the
other library is used which is kind of painful to do but it works and then the next problem
is system matters so um include why you use an clank and include cleaner have a bunch of
pragmas like what they call it to instruct the thing to do how do you say it like it's not a
foolproof algorithm right so sometimes for example when you're transitive when you want to include
one header you always want to have it exports the symbols from another file you need to use these
pragmas to say okay this header file exports this other header file and stuff like that so I mean
use that to like make the thing work basically the problem is you cannot you can only add these
pragmas to your own header files right you cannot go and modify your system libraries to have
these as well include what you use had a solution for this file configuration file where you
could add these pragmas but clank and include it doesn't really support that so it does luckily have
like a rag eggs mechanism to exclude a bunch of headers from getting reported as unused so we use
that to get around it and like the most annoying example I ran into was get up to age
and this is just a header file that does nothing but include a bunch of other header files so
it would always get marked as unused because it doesn't export any symbols and I cannot go and
fix it to add those pragmas so I have to exclude it that's slightly annoying I would prefer
that there was like some way to configure clank and clook cleaner that I could say that get up to
the age exports these headers and it would just work but that's not supported so I have to deal with it
it took me about I don't know I like it took me about a month to get all the PRS ready and merge
for removing all the in a new thing includes but then last month when I was preparing the stock
I also thought okay I should enable some more clank tidy tanks and so there are a bunch of
useful ones the argument comment like system C++ and C don't support names arguments so
what you end up doing is you end up putting an inline comment with the argument name and what clank
tidy can do is it can tell you if that comments mismatches with the actual declaration in the header
and because over time you reflect the code you rename arguments but you don't get a compilation
error for those for those comments right and clank tidy can like if you're an error when those things
start to mismatch and so I found like thousands of wrongly named argument comments which I all fixed one
by one so that there are no fixed and you get a CIR this time when you when you reflect our function
argument name and you don't update the comments readability and consistent declaration parameter
name is the same thing but for mismatches between a function declaration and it's header so the
argument names can also start to mismatch between declaration and it's definition also thousands
and thousands of errors insist indeed what that where we renamed it in the definition but on
the declarations are also when the fixed all of those so those who are also really useful to detect
all of those and to also to prevent new regressions that's the most important thing and then the
size of one was also pretty useful. The final one that we want to enable but can't
slang format so a system D is very particular about its formatting my fellow maintainers refuse
to budge right so I can't change the style I have to fix the tool so I'm slowly trying to fix
format to support our our system D formatting style what I'm currently trying to add is we
in the return types and in the cast the C-style cast we align the pointer to the left instead of the
right and other the clang format one forces you to all of them to the right of all of them to the
left so I need to make it configurable like it's a bit of a pain I'm slowly working through it but I
hope that like maybe in a year or two we'll be able to enable clang format for system D so I don't
have to manually format my code anymore. Yeah yeah they don't they don't like me adding these features
to clang format either I get a lot of pushback but like I'm stuck with between however I
hammer an envelope or something and then there was also a bunch of bugs in clang format that I still
need to fix but hopefully I'll get time to that once I will finish waiting for the build to finish
that was it I'll put answer questions
if I'm not wrong you can run clang type if I clang like in a white confine so you didn't get
it because for me I have some very experienced with that but I invite you to try this clang clang
tiny clang yeah so the remark was that there's a plug-in that you can run clang tally as a clang
plug-in so that it runs while it's compiling the code so that's basically what see make ends
of doing in its own implementation but I think like I wouldn't need to like I mean I wouldn't
need to toggle to add a toggle to only run it in various specific cases so that because most
of the build configurations you would run clang tally and it's probably not gonna it's gonna
report unused includes but it would be an option but I I don't know I might still prefer
the unit test because you might want to rerun it a few times and if the file is already compiled
and incremental compilation will prevent you from running clang tally again whereas that's always
anoint me about that approach well one more question
I use so the question is because this is a very large code based like how did I approach
like subdividing the problem so the circular includes I just started I mean we we
component or component like you don't there's no proper way to split that up there but then when I
actually and then removing transient in or sorry transdiving includes you start at the tops
of the dependency tree and then you move down and then what so but that was solely for the headers
but then when you start removing unused includes from the how do you say it from the actual
like implementation files and I mean so because removing transdiving includes it's the hard part
right you have to rewrite code you have to move function definitions around and stuff so
that's why I started with them solely for removing unused includes that's where I started
at the bottom of the tree so you start with actual damings and binaries that aren't libraries
so aren't depending on dependent on by anything else so those are easy because they only touch
themselves and then you move on to the libraries where you if you were moving unused include
from header there you don't just affect the library you also affect every consumer of the library
so that's how I handled it I think that's what I mean you can get going easily by doing the
damings because you the the piars are small but then when you get to the libraries you end up with
massive piars because you have to remove remove and add includes all over the place so that's how I handled that
yes so the question was did you notice any performance impact from moving inline functions to
a lot non-line functions by moving them to the implementation file and so no but I have to
add that our performance monitoring and system D is effectively non-existent so like it would have
been a user reported issue but we didn't get any user regression support of our performance but
like we were running blind effectively so there might have been something
that's it I think thank you