Bug 187455 – Make WebCore::Timer more space-efficient

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Simon Fraser (smfr)]

Timer member variables often introduce 7 bytes of padding because of the bool at the end.

+120 < 72>     WebCore::Timer m_timeoutTimer
+120 < 64>         WebCore::TimerBase WebCore::TimerBase
+120 <  8>            __vtbl_ptr_type * _vptr
+128 <  8>             WTF::MonotonicTime m_nextFireTime
+128 <  8>               double m_value
+136 <  8>             WTF::MonotonicTime m_unalignedNextFireTime
+136 <  8>               double m_value
+144 <  8>             WTF::Seconds m_repeatInterval
+144 <  8>               double m_value
+152 <  4>           int m_heapIndex
+156 <  4>           unsigned int m_heapInsertionOrder
+160 <  1>           bool m_wasDeleted
+161 <  7>           <PADDING: 7 bytes>

Can we steal a bit from m_heapIndex or m_heapInsertionOrder for this bool?

[Simon Fraser (smfr)]

Here's the full layout:

 4$ $ ./Tools/Scripts/dump-class-layout -c Release WebCore Timer
  +0 < 72> Timer
  +0 < 64>     WebCore::TimerBase WebCore::TimerBase
  +0 <  8>        __vtbl_ptr_type * _vptr
  +8 <  8>         WTF::MonotonicTime m_nextFireTime
  +8 <  8>           double m_value
 +16 <  8>         WTF::MonotonicTime m_unalignedNextFireTime
 +16 <  8>           double m_value
 +24 <  8>         WTF::Seconds m_repeatInterval
 +24 <  8>           double m_value
 +32 <  4>       int m_heapIndex
 +36 <  4>       unsigned int m_heapInsertionOrder
 +40 <  1>       bool m_wasDeleted
 +41 <  7>       <PADDING: 7 bytes>
 +48 <  8>       WTF::Vector<WebCore::TimerBase *, 0, WTF::CrashOnOverflow, 16> * m_cachedThreadGlobalTimerHeap
 +56 <  8>         WTF::Ref<WTF::Thread, WTF::DumbPtrTraits<WTF::Thread> > m_thread
 +56 <  8>           WTF::DumbPtrTraits<WTF::Thread>::StorageType m_ptr
 +64 <  8>     WTF::Function<void ()> m_function
 +64 <  8>         std::__1::unique_ptr<WTF::Function<void ()>::CallableWrapperBase, std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase> > m_callableWrapper
 +64 <  8>             std::__1::__compressed_pair<WTF::Function<void ()>::CallableWrapperBase *, std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase> > __ptr_
 +64 <  8>                 std::__1::__compressed_pair_elem<WTF::Function<void ()>::CallableWrapperBase *, 0, false> std::__1::__compressed_pair_elem<WTF::Function<void ()>::CallableWrapperBase *, 0, false>
 +64 <  8>                   WTF::Function<void ()>::CallableWrapperBase * __value_
 +64 <  1>                 std::__1::__compressed_pair_elem<std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase>, 1, true> std::__1::__compressed_pair_elem<std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase>, 1, true>
 +64 <  1>                     std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase> std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase>
Total byte size: 72
Total pad bytes: 7
Padding percentage: 9.72 %

[Simon Fraser (smfr)]

I think we can safely steal a bit from m_heapIndex. m_heapInsertionOrder just keeps increasing and rolls over, but stealing a bit would make that happen more often (but maybe still not often enough to care).

[Simon Fraser (smfr)]

Created attachment 346592 [details]
Patch

[Simon Fraser (smfr)]

Layout after:

 12$ $ ./Tools/Scripts/dump-class-layout -c Release WebCore Timer
  +0 < 64> Timer
  +0 < 56>     WebCore::TimerBase WebCore::TimerBase
  +0 <  8>        __vtbl_ptr_type * _vptr
  +8 <  8>         WTF::MonotonicTime m_nextFireTime
  +8 <  8>           double m_value
 +16 <  8>         WTF::MonotonicTime m_unalignedNextFireTime
 +16 <  8>           double m_value
 +24 <  8>         WTF::Seconds m_repeatInterval
 +24 <  8>           double m_value
 +32 < :31>       int m_heapIndex : 31
 +35 < :1>       bool m_wasDeleted : 1
 +36 <  4>       unsigned int m_heapInsertionOrder
 +40 <  8>       WTF::Vector<WebCore::TimerBase *, 0, WTF::CrashOnOverflow, 16> * m_cachedThreadGlobalTimerHeap
 +48 <  8>         WTF::Ref<WTF::Thread, WTF::DumbPtrTraits<WTF::Thread> > m_thread
 +48 <  8>           WTF::DumbPtrTraits<WTF::Thread>::StorageType m_ptr
 +56 <  8>     WTF::Function<void ()> m_function
 +56 <  8>         std::__1::unique_ptr<WTF::Function<void ()>::CallableWrapperBase, std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase> > m_callableWrapper
 +56 <  8>             std::__1::__compressed_pair<WTF::Function<void ()>::CallableWrapperBase *, std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase> > __ptr_
 +56 <  8>                 std::__1::__compressed_pair_elem<WTF::Function<void ()>::CallableWrapperBase *, 0, false> std::__1::__compressed_pair_elem<WTF::Function<void ()>::CallableWrapperBase *, 0, false>
 +56 <  8>                   WTF::Function<void ()>::CallableWrapperBase * __value_
 +56 <  1>                 std::__1::__compressed_pair_elem<std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase>, 1, true> std::__1::__compressed_pair_elem<std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase>, 1, true>
 +56 <  1>                     std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase> std::__1::default_delete<WTF::Function<void ()>::CallableWrapperBase>
Total byte size: 64
Total pad bytes: 0

[Brent Fulgham]

Comment on attachment 346592 [details]
Patch

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

> Source/WebCore/platform/Timer.h:101
> +    signed int m_heapIndex : 31; // -1 if not in heap

We basically lose one bit for sign, I wonder if we could pack the “not in heap” state as a bool, too, to avoid magic number code,

> Source/WebCore/platform/Timer.h:102
> +    bool m_wasDeleted : 1;

Nice!

[WebKit Commit Bot]

Comment on attachment 346592 [details]
Patch

Clearing flags on attachment: 346592

Committed r234581: <https://trac.webkit.org/changeset/234581>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/42937768>