Changeset 254509 in webkit


Ignore:
Timestamp:
Jan 14, 2020 7:22:20 AM (4 years ago)
Author:
aboya@igalia.com
Message:

[WTF] Make MediaTime constructor constexpr
https://bugs.webkit.org/show_bug.cgi?id=206036

Reviewed by Adrian Perez de Castro.

Source/WTF:

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.

  • wtf/MediaTime.cpp:
  • wtf/MediaTime.h:

(WTF::MediaTime::MediaTime):

Tools:

Added test for global static MediaTime constants.

  • TestWebKitAPI/Tests/WTF/MediaTime.cpp:

(TestWebKitAPI::TEST):

Location:
trunk
Files:
5 edited

Legend:

Unmodified
Added
Removed
  • trunk/Source/WTF/ChangeLog

    r254500 r254509  
     12020-01-14  Alicia Boya García  <aboya@igalia.com>
     2
     3        [WTF] Make MediaTime constructor constexpr
     4        https://bugs.webkit.org/show_bug.cgi?id=206036
     5
     6        Reviewed by Adrian Perez de Castro.
     7
     8        https://bugs.webkit.org/show_bug.cgi?id=205723 allowed to declare
     9        MediaTime variables as static inside functions without needing a
     10        global destructor.
     11
     12        It did not eliminate the call to the MediaTime constructor on runtime
     13        though. This wasn't a problem for static variables inside functions,
     14        as the compiler adds a guard variable to call the constructor the
     15        first time the function is called.
     16
     17        On the other hand, for variables defined outside of the scope of the
     18        function, for them to be initialized the MediaTime constructor would
     19        have to be called at runtime from a global constructor, something
     20        we're trying to avoid and which generates an error in clang.
     21
     22        But again, MediaTime is a simple class with only integral values, we
     23        shouldn't need a runtime function call to initialize it!
     24
     25        This patch makes the MediaTime constructor constexpr so that we don't
     26        need runtime initialization for static MediaTime variables. This
     27        allows us to declare them outside functions and enables the compiler
     28        to generate code without guard variables when static MediaTime
     29        variables are declared inside functions.
     30
     31        A test has been added accessing a global const static MediaTime. The
     32        build should not produce any error stating the need for a global
     33        constructor.
     34
     35        * wtf/MediaTime.cpp:
     36        * wtf/MediaTime.h:
     37        (WTF::MediaTime::MediaTime):
     38
    1392020-01-14  David Kilzer  <ddkilzer@apple.com>
    240
  • trunk/Source/WTF/wtf/MediaTime.cpp

    r254200 r254509  
    7373const uint32_t MediaTime::MaximumTimeScale = 1000000000;
    7474
    75 MediaTime::MediaTime()
    76     : m_timeValue(0)
    77     , m_timeScale(DefaultTimeScale)
    78     , m_timeFlags(Valid)
    79 {
    80 }
    81 
    82 MediaTime::MediaTime(int64_t value, uint32_t scale, uint8_t flags)
    83     : m_timeValue(value)
    84     , m_timeScale(scale)
    85     , m_timeFlags(flags)
    86 {
    87     if (scale || isInvalid())
    88         return;
    89 
    90     *this = value < 0 ? negativeInfiniteTime() : positiveInfiniteTime();
    91 }
    92 
    9375MediaTime::MediaTime(const MediaTime& rhs)
    9476{
  • trunk/Source/WTF/wtf/MediaTime.h

    r254200 r254509  
    5454    };
    5555
    56     MediaTime();
    57     MediaTime(int64_t value, uint32_t scale, uint8_t flags = Valid);
     56    constexpr MediaTime();
     57    constexpr MediaTime(int64_t value, uint32_t scale, uint8_t flags = Valid);
    5858    MediaTime(const MediaTime& rhs);
    5959
     
    149149};
    150150
     151constexpr MediaTime::MediaTime()
     152    : m_timeValue(0)
     153    , m_timeScale(DefaultTimeScale)
     154    , m_timeFlags(Valid)
     155{
     156}
     157
     158constexpr MediaTime::MediaTime(int64_t value, uint32_t scale, uint8_t flags)
     159    : m_timeValue(value)
     160    , m_timeScale(scale)
     161    , m_timeFlags(flags)
     162{
     163    if (scale || !(flags & Valid))
     164        return;
     165
     166    *this = value < 0 ? negativeInfiniteTime() : positiveInfiniteTime();
     167}
     168
    151169inline MediaTime operator*(int32_t lhs, const MediaTime& rhs) { return rhs.operator*(lhs); }
    152170
  • trunk/Tools/ChangeLog

    r254493 r254509  
     12020-01-14  Alicia Boya García  <aboya@igalia.com>
     2
     3        [WTF] Make MediaTime constructor constexpr
     4        https://bugs.webkit.org/show_bug.cgi?id=206036
     5
     6        Reviewed by Adrian Perez de Castro.
     7
     8        Added test for global static MediaTime constants.
     9
     10        * TestWebKitAPI/Tests/WTF/MediaTime.cpp:
     11        (TestWebKitAPI::TEST):
     12
    1132020-01-13  Fujii Hironori  <Hironori.Fujii@sony.com>
    214
  • trunk/Tools/TestWebKitAPI/Tests/WTF/MediaTime.cpp

    r239688 r254509  
    5656
    5757namespace TestWebKitAPI {
     58
     59// MediaTime values should be able to be declared static anywhere, just like you can do so with integers.
     60// This should not require global constructors or destructors.
     61#pragma clang diagnostic push
     62#pragma clang diagnostic error "-Wglobal-constructors"
     63static const MediaTime oneSecond(1, 1);
     64#pragma clang diagnostic pop
    5865
    5966TEST(WTF, MediaTime)
     
    98105    EXPECT_FALSE(!MediaTime(1, 1));
    99106    EXPECT_FALSE((bool)MediaTime::invalidTime());
     107    EXPECT_EQ(MediaTime(10, 10), oneSecond);
    100108
    101109    // Addition Operators
Note: See TracChangeset for help on using the changeset viewer.