libsdl-org / SDL

Simple Directmedia Layer
https://libsdl.org
zlib License
9.77k stars 1.81k forks source link

SDL3 build time increases... #6704

Closed icculus closed 1 year ago

icculus commented 1 year ago

Laptop with 4 physical CPU cores:

The configure script, SDL2, debug build: CFLAGS="-O0 -ggdb3" ../configure

time make -j8 reports

real    0m35.094s

CMake, SDL2, debug build: CFLAGS="-O0 -ggdb3" cmake -G Ninja ..

time ninja reports

real    0m29.715s

CMake, SDL3, debug build: CFLAGS="-O0 -ggdb3" cmake -G Ninja ..

time ninja reports

real    0m52.708s

What did we do that almost doubled the build time?

slouken commented 1 year ago

We included all SDL headers instead of just the ones needed in each source file, in https://github.com/libsdl-org/SDL/commit/0a48abc860b436387c94e90c13fd0db25772e356.

I did that partially to simplify the header migration and partially because the set of headers included wasn't included in order or always the minimal set. Is there a header analysis tool that can automate getting the minimum compilable set of headers?

Precompiled header support for SDL_internal.h would be fantastic, and greatly reduce build time everywhere. :)

oleg-derevenetz commented 1 year ago

@slouken

Is there a header analysis tool that can automate getting the minimum compilable set of headers?

We use include-what-you-use (IWYU) in our project. It works pretty well with in-project headers and "good enough" with system headers, but it is targeted for C++, not C. It should work with C too, I believe, but I never tried to use it in pure C projects.

madebr commented 1 year ago

I tried target_precompile_headers at https://github.com/madebr/SDL/tree/precompiled-headers, but am not seeing a reduction in build time.

1bsyl commented 1 year ago

@slouken I've tried to revert partially your patch on src/ and test/ + modify the header to keep the one before. not sure if this is faster because I already have a recent machine ... patch_include.txt

slouken commented 1 year ago

FWIW, SDL2 build times: real 1m48.792s SDL3 build times: real 3m11.533s SDL3 build times with precompiled header: real 2m12.512s

So including the minimal set of headers really does give the best compile times.

madebr commented 1 year ago

I revert my last statement: ccache was enabled. Build time goes from 5m25s to 1m18s with precompiled headers SDL2 build time is 2m44.

slouken commented 1 year ago

I revert my last statement: ccache was enabled. Build time goes from 5m25s to 1m18s with precompiled headers SDL2 build time is 2m44.

Wow, your change is great, can you please merge it?

On my system, build times dropped to: real 1m6.407s

madebr commented 1 year ago

Will do. But for CMake 3.16+ only.

madebr commented 1 year ago

Also keep in mind that this forces SDL_internal.h to be included, even when you don't add it as an #include "../SDL_internal.h".

This is why it is explicitly disabled for src/dynapi/SDL_dynapi.c. It renamed the SDL_xxx symbols to SDL_xxx_REAL, causing duplicate symbol link errors.

slouken commented 1 year ago

That's fine. I've actually add it to all the headers and source files manually anyway. That's the intent of that file in SDL builds.

madebr commented 1 year ago

https://github.com/libsdl-org/SDL/pull/6708 so CI can have a say.

icculus commented 1 year ago

Um, I went from 52 seconds to 19.

Holy moly, I think we can call this resolved. :)

slime73 commented 1 year ago

Is there anything that needs to be done for the visual studio and xcode projects?

madebr commented 1 year ago

target_precompile_headers mentions MSVC. I don't know about xcode though.

icculus commented 1 year ago

I'll do a comparison real fast, sit tight.

slime73 commented 1 year ago

target_precompile_headers mentions MSVC. I don't know about xcode though.

I was mostly thinking of the existing project files (https://github.com/libsdl-org/SDL/issues/6571#issuecomment-1325657754) - but either way I don't think build times are as important for them as the cmake project, I'm just wondering if they take significantly longer now and if so whether there are any easy changes to get them closer to cmake's times.

icculus commented 1 year ago

This M1 Mac Mini, using xcodebuild with the CMake project, with the commit before precompiled headers:

real    0m9.768s

The commit that enabled precompiled headers:

real    0m6.497s

Shaving off 1/3rd of the time isn't that impressive when we went from 9 to 6 seconds, but obviously older Intel machines are likely to benefit more here.

So I think the answer is, yes, let's absolutely find the checkbox to enable this in the standalone Xcode project, too.

slouken commented 1 year ago

I'm working on this now.

icculus commented 1 year ago

@slouken: For xcode, you set this:

image

"whatever.h" needs to be something it can find, I don't know how to specify a relative path or if there's a $(PROJECT_DIR) variable or something.

It can't point right to SDL_internal.h, because SDL_dynapi.c can't use precompiled headers. CMake generates a stub file and does this:

/* generated by CMake */

#ifndef CMAKE_SKIP_PRECOMPILE_HEADERS
#include "/Users/icculus/projects/SDL/src/SDL_internal.h"
#endif // CMAKE_SKIP_PRECOMPILE_HEADERS

And then defines CMAKE_SKIP_PRECOMPILE_HEADERS on the compiler command line for just SDL_dynapi.c

(Obviously that doesn't need to be an absolute path, but that's just how CMake rolls.)

I don't know what Visual Studio needs.

slouken commented 1 year ago

New Xcode is even easier. You just enable modules and Xcode automatically converts all #include statements into precompiled header directives. I flipped that on and compile times went from 12 seconds to 7 seconds - even faster than CMake's 8 seconds with precompiled headers enabled. :)