187455 2018-07-08 19:20:37 -0700 Make WebCore::Timer more space-efficient 2018-08-04 15:58:17 -0700 1 1 1 Unclassified WebKit WebCore Misc. WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 simon.fraser simon.fraser bfulgham cdumez commit-queue darin sam simon.fraser webkit-bug-importer oldest_to_newest 1440275 0 simon.fraser 2018-07-08 19:20:37 -0700 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? 1440276 1 simon.fraser 2018-07-08 19:27:38 -0700 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 % 1440670 2 simon.fraser 2018-07-09 18:01:27 -0700 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). 1447925 3 346592 simon.fraser 2018-08-04 11:32:44 -0700 Created attachment 346592 Patch 1447927 4 simon.fraser 2018-08-04 11:33:46 -0700 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 1447963 5 346592 bfulgham 2018-08-04 15:30:30 -0700 Comment on attachment 346592 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! 1447968 6 346592 commit-queue 2018-08-04 15:57:20 -0700 Comment on attachment 346592 Patch Clearing flags on attachment: 346592 Committed r234581: <https://trac.webkit.org/changeset/234581> 1447969 7 commit-queue 2018-08-04 15:57:22 -0700 All reviewed patches have been landed. Closing bug. 1447970 8 webkit-bug-importer 2018-08-04 15:58:17 -0700 <rdar://problem/42937768> 346592 2018-08-04 11:32:44 -0700 2018-08-04 15:57:20 -0700 Patch bug-187455-20180804113244.patch text/plain 2207 simon.fraser U3VidmVyc2lvbiBSZXZpc2lvbjogMjM0NTc5CmRpZmYgLS1naXQgYS9Tb3VyY2UvV2ViQ29yZS9D aGFuZ2VMb2cgYi9Tb3VyY2UvV2ViQ29yZS9DaGFuZ2VMb2cKaW5kZXggNDUyNGQ0MjE5ZGZiYWEx N2ViYTc3MGMwOGRkN2IzMzNlNDdkYmU4Yy4uZTZjYTc1YmMwY2VjODU4ZWI3NjQzN2M1Njg0ZDI3 OTVjZDUxN2VmMSAxMDA2NDQKLS0tIGEvU291cmNlL1dlYkNvcmUvQ2hhbmdlTG9nCisrKyBiL1Nv dXJjZS9XZWJDb3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE3IEBACisyMDE4LTA4LTA0ICBTaW1v biBGcmFzZXIgIDxzaW1vbi5mcmFzZXJAYXBwbGUuY29tPgorCisgICAgICAgIE1ha2UgV2ViQ29y ZTo6VGltZXIgbW9yZSBzcGFjZS1lZmZpY2llbnQKKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtp dC5vcmcvc2hvd19idWcuY2dpP2lkPTE4NzQ1NQorCisgICAgICAgIFJldmlld2VkIGJ5IE5PQk9E WSAoT09QUyEpLgorCisgICAgICAgIFN0ZWFsIGEgYml0IGZyb20gbV9oZWFwSW5kZXggZm9yIG1f d2FzRGVsZXRlZCB0byBlbGltaW5hdGUgcGFkZGluZyBpbiBUaW1lci4KKyAgICAgICAgVGhpcyBy ZWR1Y2VzIHRoZSBzaXplIG9mIERvY3VtZW50LCB3aGljaCBoYXMgOCBUaW1lcnMsIGZyb20gMzM2 MCB0byAzMjk2IGJ5dGVzIChzYXZpbmcgNjQgYnl0ZXMpLgorCisgICAgICAgICogcGxhdGZvcm0v VGltZXIuY3BwOgorICAgICAgICAoV2ViQ29yZTo6VGltZXJCYXNlOjpUaW1lckJhc2UpOgorICAg ICAgICAqIHBsYXRmb3JtL1RpbWVyLmg6CisKIDIwMTgtMDgtMDQgIFJ5b3N1a2UgTml3YSAgPHJu aXdhQHdlYmtpdC5vcmc+CiAKICAgICAgICAgUHJvcGVydGllcyBzZXQgb24gd2luZG93LmN1c3Rv bUVsZW1lbnRzIGNhbiBkaXNhcHBlYXIgZHVlIHRvIEdDCmRpZmYgLS1naXQgYS9Tb3VyY2UvV2Vi Q29yZS9wbGF0Zm9ybS9UaW1lci5jcHAgYi9Tb3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9UaW1lci5j cHAKaW5kZXggNTcwNmMyZThlYjZhODQ1Y2I4YmNkN2E0NzYyYTljNTQ0OGZkZmI5My4uNzIyZDQ1 MzlmZTFmMjY4ZmVkZWJhNjEwMDUwZDcwODEzZWJmNWJmMiAxMDA2NDQKLS0tIGEvU291cmNlL1dl YkNvcmUvcGxhdGZvcm0vVGltZXIuY3BwCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3JtL1Rp bWVyLmNwcApAQCAtMTg1LDYgKzE4NSw4IEBAIGlubGluZSBib29sIFRpbWVySGVhcExlc3NUaGFu RnVuY3Rpb246Om9wZXJhdG9yKCkoY29uc3QgVGltZXJCYXNlKiBhLCBjb25zdCBUaW1lCiAvLyAt LS0tLS0tLS0tLS0tLS0tCiAKIFRpbWVyQmFzZTo6VGltZXJCYXNlKCkKKyAgICA6IG1faGVhcElu ZGV4KC0xKQorICAgICwgbV93YXNEZWxldGVkKGZhbHNlKQogewogfQogCmRpZmYgLS1naXQgYS9T b3VyY2UvV2ViQ29yZS9wbGF0Zm9ybS9UaW1lci5oIGIvU291cmNlL1dlYkNvcmUvcGxhdGZvcm0v VGltZXIuaAppbmRleCA1MjRkNDg2MWYyYjQ4ZDMwNDU3YmJmMDM3ZmI1NTQxNDk5ZjUwNzcyLi5l YzkwYjg2MWQ5MmEyODgzYjlhZmEwODY5MGY2MDZhZDhlNDIwMWRmIDEwMDY0NAotLS0gYS9Tb3Vy Y2UvV2ViQ29yZS9wbGF0Zm9ybS9UaW1lci5oCisrKyBiL1NvdXJjZS9XZWJDb3JlL3BsYXRmb3Jt L1RpbWVyLmgKQEAgLTk4LDkgKzk4LDkgQEAgcHJpdmF0ZToKICAgICBNb25vdG9uaWNUaW1lIG1f bmV4dEZpcmVUaW1lOyAvLyAwIGlmIGluYWN0aXZlCiAgICAgTW9ub3RvbmljVGltZSBtX3VuYWxp Z25lZE5leHRGaXJlVGltZTsgLy8gbV9uZXh0RmlyZVRpbWUgbm90IGNvbnNpZGVyaW5nIGFsaWdu bWVudCBpbnRlcnZhbAogICAgIFNlY29uZHMgbV9yZXBlYXRJbnRlcnZhbDsgLy8gMCBpZiBub3Qg cmVwZWF0aW5nCi0gICAgaW50IG1faGVhcEluZGV4IHsgLTEgfTsgLy8gLTEgaWYgbm90IGluIGhl YXAKKyAgICBzaWduZWQgaW50IG1faGVhcEluZGV4IDogMzE7IC8vIC0xIGlmIG5vdCBpbiBoZWFw CisgICAgYm9vbCBtX3dhc0RlbGV0ZWQgOiAxOwogICAgIHVuc2lnbmVkIG1faGVhcEluc2VydGlv bk9yZGVyOyAvLyBVc2VkIHRvIGtlZXAgb3JkZXIgYW1vbmcgZXF1YWwtZmlyZS10aW1lIHRpbWVy cwotICAgIGJvb2wgbV93YXNEZWxldGVkIHsgZmFsc2UgfTsKICAgICBWZWN0b3I8VGltZXJCYXNl Kj4qIG1fY2FjaGVkVGhyZWFkR2xvYmFsVGltZXJIZWFwIHsgbnVsbHB0ciB9OwogCiAgICAgUmVm PFRocmVhZD4gbV90aHJlYWQgeyBUaHJlYWQ6OmN1cnJlbnQoKSB9Owo=