Compile times and class-scoped enums
Hi all! I’ve noticed that compile times for the WebKit project have started creeping up again, so I and a few other WebKit contributors started looking into possible compile time improvements using the tools listed in <https://trac.webkit.org/wiki/AnalyzingBuildPerformance>. One cause of long compile times is when a commonly-included header (like Document.h) includes other headers (which include other headers, ad nauseam). Whereas much of the time those includes can be avoided by forward declaring types, for types (specifically enums) scoped within classes, this is impossible. I attempted to address this in <https://github.com/WebKit/WebKit/pull/8867>, which (on this machine) reduces the total compile time of Document.h in the WebCore project from about 1000s to about 200s. However, this change requires moving class-scoped enums out into the namespace scope. E.g.:
diff --git a/Source/WebCore/Modules/audiosession/DOMAudioSession.h b/Source/WebCore/Modules/audiosession/DOMAudioSession.h index 01bf6960d3a4..d84e1eae78d5 100644 --- a/Source/WebCore/Modules/audiosession/DOMAudioSession.h +++ b/Source/WebCore/Modules/audiosession/DOMAudioSession.h @@ -36,14 +36,17 @@
namespace WebCore {
+enum class DOMAudioSessionType : uint8_t { Auto, Playback, Transient, TransientSolo, Ambient, PlayAndRecord }; +enum class DOMAudioSessionState : uint8_t { Inactive, Active, Interrupted }; + class DOMAudioSession final : public RefCounted<DOMAudioSession>, public ActiveDOMObject, public EventTarget, public AudioSession::InterruptionObserver { WTF_MAKE_ISO_ALLOCATED(DOMAudioSession); public: static Ref<DOMAudioSession> create(ScriptExecutionContext*); ~DOMAudioSession();
- enum class Type : uint8_t { Auto, Playback, Transient, TransientSolo, Ambient, PlayAndRecord }; - enum class State : uint8_t { Inactive, Active, Interrupted }; + using Type = DOMAudioSessionType; + using State = DOMAudioSessionState;
ExceptionOr<void> setType(Type); Type type() const;
So that these enums can be forward declared in Document.h, rather than including the header wholesale. However, this requires a significant coding style change, both to existing code and new code, and as such, it should probably be discussed here. So, what do people think? Is the change in coding style (moving class-scoped enums out into namespace scope) worth doing if it means a significant increases in compile speeds? -Jer
On Fri, Jan 20 2023 at 09:50:05 AM -0800, Jer Noble via webkit-dev <webkit-dev@lists.webkit.org> wrote:
However, this requires a significant coding style change, both to existing code and new code, and as such, it should probably be discussed here. So, what do people think? Is the change in coding style (moving class-scoped enums out into namespace scope) worth doing if it means a significant increases in compile speeds?
My $0.02: it's awful but worth it. That's a ridiculous speedup. Nice.
On Jan 20, 2023, at 9:50 AM, Jer Noble via webkit-dev <webkit-dev@lists.webkit.org> wrote:
I attempted to address this in <https://github.com/WebKit/WebKit/pull/8867>, which (on this machine) reduces the total compile time of Document.h in the WebCore project from about 1000s to about 200s.
That’s good.
However, this change requires moving class-scoped enums out into the namespace scope.
Seems worthwhile. Doesn’t seem to me like it would have far reaching effects. — Darin
However, this change requires moving class-scoped enums out into the namespace scope.
Seems worthwhile. Doesn’t seem to me like it would have far reaching effects.
I agree.
+ using Type = DOMAudioSessionType;
Did you do this to make the patch smaller, or do you prefer this style? Thanks, Geoff
On Jan 23, 2023, at 11:05 AM, Geoffrey Garen <ggaren@apple.com> wrote:
However, this change requires moving class-scoped enums out into the namespace scope.
Seems worthwhile. Doesn’t seem to me like it would have far reaching effects.
I agree.
+ using Type = DOMAudioSessionType;
Did you do this to make the patch smaller, or do you prefer this style?
Yes to both. IMO, there’s nothing inherently wrong with having an enum scoped to a class apart from that makes it impossible to forward declare. If there was some language supported mechanism to forward-declare class-scoped enums, we’d probably just do that instead. So, this pattern of declaring the enum outside of a class and pulling it into the class with a using declaration is a bit ugly, insofar as it pollutes the global namespace, but it does allow you to continue to use the type as if it were file scoped. -Jer
participants (4)
-
Darin Adler
-
Geoffrey Garen
-
Jer Noble
-
Michael Catanzaro