Actually we have only two remaining usages of such macros in blink geometry types: 1. float_size.h <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/geometry/float_size.h;drc=134f570f0aff185007cd8a2757f4188567a7df9f;l=240> : // Allows this class to be stored in a HeapVector. WTF_ALLOW_CLEAR_UNUSED_SLOTS_WITH_MEM_FUNCTIONS(blink::FloatSize) This seems unnecessary because FloatSize is not a garbage collected type, and we don't have any usage of Vector<FloatSize> in blink. 2. int_rect.h <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/geometry/int_rect.h;drc=134f570f0aff185007cd8a2757f4188567a7df9f;l=272> : WTF_ALLOW_MOVE_INIT_AND_COMPARE_WITH_MEM_FUNCTIONS(blink::IntRect) We have a lot of Vector<IntRect> usages. Will try perf tests.
About std::is_trivial, actually we don't allow particular trivial types to use such macros. For example, in WTF_ALLOW_MOVE_AND_INIT_WITH_MEM_FUNCTIONS, we have: static_assert(!std::is_trivially_default_constructible<ClassName>::value || \ !std::is_trivially_move_assignable<ClassName>::value, \ "macro not needed"); \ Also most of our classes (including all geometry classes) are not trivially default constructible because they do need initialization. We could check if a default constructed object is all zero to see if it can be initialized by memset, but I'm not sure if this is possible at compile time. It seems that we can check std::is_trivially_copyable and/or std::is_trivially_move_assignable for kCan*WithMemcpy and kCanCompareWithMemcmp. On Thu, Nov 4, 2021 at 10:39 AM Xianzhu Wang <[email protected]> wrote: > One data point: the CL > <https://chromium-review.googlesource.com/c/chromium/src/+/3252774> that > replaces blink::IntPoint (declared with the macro) with gfx::Point was > landed 2 days ago, and I haven't received any performance bug yet. Perhaps > we don't use Vector<IntPoint> in performance critical code. > > Will try std::is_trivial before converting other types. > > On Thu, Nov 4, 2021 at 10:19 AM Daniel Bratell <[email protected]> > wrote: > >> (see inline) >> On 2021-11-03 12:15, Kentaro Hara wrote: >> >> The VectorTraits<T>::kCan{Copy,Move,Fill,Compare}... fields are used for >>> all Vector<T>s though, no? This particular macro doesn't appear to >>> explicitly define/override any Oilpan-related fields AFAICS. >> >> >> You're right -- thanks for pointing it out :) >> >> a) The macros work as a performance optimization for all Vector<T>s. I >> have no data about the performance numbers though. >> >> The traits have a big effect on micro-benchmarks but it's always hard to >> map that to user observable performance. >> >> Furthermore, is it possible to replace the macros with use of standard >> C++ traits such as std::is_trivial >> <http://www.cplusplus.com/reference/type_traits/is_trivial/>? It would >> require auditing current types, and possibly modifying them, but that is >> something that has to be done anyway before a complete type merge between >> Blink and >> >> /Daniel >> >> >> b) Some of the macros are mandatory to make HeapVector<T> work correctly >> (e.g., see static_assert here >> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/heap/collection_support/heap_vector_backing.h;l=81?q=kCanClearUnusedSlotsWithMemset&ss=chromium%2Fchromium%2Fsrc:third_party%2Fblink%2Frenderer%2Fplatform%2F> >> ). >> >> b) does not apply to the geometry types in question. Regarding a), maybe >> can we just measure performance using benchmarks and see if dropping the >> macros causes performance regressions? (I hope not.) >> >> >> On Wed, Nov 3, 2021 at 7:19 PM Fredrik Söderquist <[email protected]> wrote: >> >>> On Wed, Nov 3, 2021 at 12:29 AM Kentaro Hara <[email protected]> >>> wrote: >>> >>>> The macro matters only for objects stored in HeapVector<T> (i.e. >>>> Oilpan). So you don't need to export the macro outside Blink :) >>>> >>> >>> The VectorTraits<T>::kCan{Copy,Move,Fill,Compare}... fields are used for >>> all Vector<T>s though, no? This particular macro doesn't appear to >>> explicitly define/override any Oilpan-related fields AFAICS. >>> >>> >>> /fs >>> >>> >>>> >>>> >>>> 2021年11月3日(水) 8:04 Xianzhu Wang <[email protected]>: >>>> >>>>> Hi, >>>>> >>>>> I'm migrating blink to use gfx geometry types, and noticed >>>>> that WTF_ALLOW_MOVE_INIT_AND_COMPARE_WITH_MEM_FUNCTIONS is applied to >>>>> blink >>>>> geometry types. It seems to optimize performance of Vector<Type>. >>>>> >>>>> I have the following questions: >>>>> >>>>> 1. For a type defined out of blink, how do we force users to include a >>>>> header delaring WTF_ALLOW_MOVE_INIT_AND_COMPARE_WITH_MEM_FUNCTIONS(Type)? >>>>> I'm thinking of putting it in vector_traits.h >>>>> <https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/wtf/vector_traits.h>, >>>>> but I would like to know if there are better ways. >>>>> >>>>> 2. Do we have data showing the performance difference with and without >>>>> WTF_ALLOW_MOVE_INIT_AND_COMPARE_WITH_MEM_FUNCTIONS? >>>>> >>>>> Thanks, >>>>> Xianzhu >>>>> >>>>> -- >>>>> You received this message because you are subscribed to the Google >>>>> Groups "blink-dev" group. >>>>> To unsubscribe from this group and stop receiving emails from it, send >>>>> an email to [email protected]. >>>>> To view this discussion on the web visit >>>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADBxriepZahOVJfzQseAyFfkjUPUgLWovXrKUYH54UGY6K2mEw%40mail.gmail.com >>>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADBxriepZahOVJfzQseAyFfkjUPUgLWovXrKUYH54UGY6K2mEw%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>>> . >>>>> >>>> -- >>>> You received this message because you are subscribed to the Google >>>> Groups "blink-dev" group. >>>> To unsubscribe from this group and stop receiving emails from it, send >>>> an email to [email protected]. >>>> To view this discussion on the web visit >>>> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jxn-cipEGf%2BU04WVa3UNqLmqmFcX55dWCPy_KK6huAe3A%40mail.gmail.com >>>> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jxn-cipEGf%2BU04WVa3UNqLmqmFcX55dWCPy_KK6huAe3A%40mail.gmail.com?utm_medium=email&utm_source=footer> >>>> . >>>> >>> >> >> -- >> Kentaro Hara, Tokyo >> -- >> You received this message because you are subscribed to the Google Groups >> "blink-dev" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jxmhADNpdOCAnq_VbH-R%2BVbhJc%3DvS2CAW4LQv10OukU6w%40mail.gmail.com >> <https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CABg10jxmhADNpdOCAnq_VbH-R%2BVbhJc%3DvS2CAW4LQv10OukU6w%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> >> -- You received this message because you are subscribed to the Google Groups "blink-dev" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/blink-dev/CADBxrifaau1OH2rijz-j3fMC79rGeu3OpkYGLLMPj1wLRMqCtA%40mail.gmail.com.
