Bug 206036 - [WTF] Make MediaTime constructor constexpr
Summary: [WTF] Make MediaTime constructor constexpr
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Web Template Framework (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alicia Boya García
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2020-01-09 14:13 PST by Alicia Boya García
Modified: 2020-01-14 07:56 PST (History)
14 users (show)

See Also:


Attachments
Patch (5.91 KB, patch)
2020-01-09 16:10 PST, Alicia Boya García
no flags Details | Formatted Diff | Diff
Patch (5.92 KB, patch)
2020-01-13 08:07 PST, Alicia Boya García
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alicia Boya García 2020-01-09 14:13:36 PST
https://bugs.webkit.org/show_bug.cgi?id=205723 allowed to declare
MediaTime variables as static inside functions without needing a
global destructor.

It did not eliminate the call to the MediaTime constructor on runtime
though. This wasn't a problem for static variables inside functions,
as the compiler adds a guard variable to call the constructor the
first time the function is called.

On the other hand, for variables defined outside of the scope of the
function, for them to be initialized the MediaTime constructor would
have to be called at runtime from a global constructor, something
we're trying to avoid and which generates an error in clang.

But again, MediaTime is a simple class with only integral values, we
shouldn't need a runtime function call to initialize it!

This patch makes the MediaTime constructor constexpr so that we don't
need runtime initialization for static MediaTime variables. This
allows us to declare them outside functions and enables the compiler
to generate code without guard variables when static MediaTime
variables are declared inside functions.

A test has been added accessing a global const static MediaTime. The
build should not produce any error stating the need for a global
constructor.
Comment 1 Alicia Boya García 2020-01-09 16:10:18 PST
Created attachment 387286 [details]
Patch
Comment 2 Alicia Boya García 2020-01-13 08:07:33 PST
Created attachment 387527 [details]
Patch
Comment 3 Adrian Perez 2020-01-14 06:09:45 PST
Comment on attachment 387527 [details]
Patch

Nice one :)
Comment 4 WebKit Commit Bot 2020-01-14 07:21:50 PST
The commit-queue encountered the following flaky tests while processing attachment 387527 [details]:

editing/spelling/spellcheck-async-remove-frame.html bug 158401 (authors: morrita@google.com, rniwa@webkit.org, and tony@chromium.org)
The commit-queue is continuing to process your patch.
Comment 5 WebKit Commit Bot 2020-01-14 07:22:25 PST
Comment on attachment 387527 [details]
Patch

Clearing flags on attachment: 387527

Committed r254509: <https://trac.webkit.org/changeset/254509>
Comment 6 WebKit Commit Bot 2020-01-14 07:22:26 PST
All reviewed patches have been landed.  Closing bug.
Comment 7 Radar WebKit Bug Importer 2020-01-14 07:23:14 PST
<rdar://problem/58567175>
Comment 8 David Kilzer (:ddkilzer) 2020-01-14 07:35:57 PST
Comment on attachment 387527 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387527&action=review

> Source/WTF/ChangeLog:15
> +        It did not eliminate the call to the MediaTime constructor on runtime
> +        though. This wasn't a problem for static variables inside functions,
> +        as the compiler adds a guard variable to call the constructor the
> +        first time the function is called.

Note that Apple ports DISABLE thread-safe static variable initialization for performance reasons, so this statement may not be true for all ports.

This Xcode variable is turned into a compiler switch that disables them:

    GCC_THREADSAFE_STATICS = NO;
Comment 9 Alicia Boya García 2020-01-14 07:56:15 PST
Comment on attachment 387527 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=387527&action=review

>> Source/WTF/ChangeLog:15
>> +        first time the function is called.
> 
> Note that Apple ports DISABLE thread-safe static variable initialization for performance reasons, so this statement may not be true for all ports.
> 
> This Xcode variable is turned into a compiler switch that disables them:
> 
>     GCC_THREADSAFE_STATICS = NO;

Well, the guard variable would (if it were not for this patch) still be there, as the initialization would still be delayed to runtime... it would just be checked and set in a non thread-safe way.