Skip to content

Commit

Permalink
Revert of Expanding Type Traits to make Vector use mem ops more. (pat…
Browse files Browse the repository at this point in the history 
…chset #4 id:120001 of https://codereview.chromium.org/581683002/)

Reason for revert:
Broke developer builds on os x: https://code.google.com/p/chromium/issues/detail?id=416715

Original issue's description:
> Expanding Type Traits to make Vector use mem ops more.
> 
> Vector has several code paths depending on whether the encapsulated
> object can be handled with memmove/memcpu/memset or not, and those
> are used by many objects. Unfortunately that requires manual hinting
> in each class because the type traits used are not strong enough.
> 
> By using type trait functions available in all modern compilers it's
> possible to more intelligently select the most optimal code path.
> 
> Preferably the code would use the new type traits in C++ 11 but
> unfortunately that requires a modern c++ library. Luckily, those
> libraries are mostly just thin wrappers on top of de facto-standard
> compiler extension functions anyway.
> 
> With clang this cuts away 105 KB of machine code in blink. Mostly by 
> no longer needing copy constructor and assignment operators in many
> classes so those can be stripped from the binary. 
> 
> With gcc the changes are differently. The binary is smaller but only 
> by 5 KB. It's clear that many Vector methods have become more compact but
> it seems gcc compensated by spending that space elsewhere (more inlining?).
> 
> I don't have as good measurement tools for Visual Studio but there was no
> huge difference in footprint.
> 
> Performance, all this is from a noisy development computer with a 
> non-stable cpu clock frequency (turbo ftl...):
> 
> In LayoutTests most changes are within noise levels. But it looks like 
> large tables and flexboxes might be 1-5% faster. In ParserTests 
> html5-full-render (macro-benchmark) became 2-3% faster.
> 
> clang footprint numbers:
> Total change: -109940 bytes
> ===========================
>   14 added, totalling +3397 bytes across 8 sources
>   19 removed, totalling -13302 bytes across 6 sources
>   8 grown, for a net change of +578 bytes (2268 bytes before, 2846 bytes after) across 6 sources
>   230 shrunk, for a net change of -100613 bytes (190909 bytes before, 90296 bytes after) across 49 sources
> 
> Biggest changes:
>  -84410 - Source: /home/bratell/src/chromium/src/third_party/WebKit/Source/wtf/Vector.h - (gained 784, lost 85194)
>   -6934 - Source: /home/bratell/src/chromium/src/third_party/WebKit/Source/wtf/Deque.h - (gained 0, lost 6934)
>   -2267 - Source: /home/bratell/src/chromium/src/third_party/WebKit/Source/core/animation/css/CSSAnimationData.cpp - (gained 904, lost 3171)
> ...
>    +245 - Source: /home/bratell/src/chromium/src/third_party/WebKit/Source/core/svg/animation/SVGSMILElement.cpp - (gained 245, lost 0)
> 
> BUG=
> [email protected]
> 
> Committed: https://src.chromium.org/viewvc/blink?view=rev&revision=182427

[email protected],[email protected],[email protected]
NOTREECHECKS=true
NOTRY=true
BUG=

Review URL: https://codereview.chromium.org/594763002

git-svn-id: svn://svn.chromium.org/blink/trunk@182459 bbb929c8-8fbe-4397-9dbb-9b2b20218538
  • Loading branch information
@nico
nico committed Sep 23, 2014
1 parent 95ffb27 commit 463f3d2
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 145 deletions.
85 changes: 1 addition & 84 deletions Source/wtf/TypeTraits.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,23 +52,6 @@ COMPILE_ASSERT(IsFloatingPoint<double>::value, WTF_IsFloatingPoint_double_true);
COMPILE_ASSERT(IsFloatingPoint<long double>::value, WTF_IsFloatingPoint_long_double_true);
COMPILE_ASSERT(!IsFloatingPoint<int>::value, WTF_IsFloatingPoint_int_false);

COMPILE_ASSERT(IsPointer<void*>::value, WTF_IsPointer_void_star_true);
COMPILE_ASSERT(IsPointer<char*>::value, WTF_IsPointer_char_star_true);
COMPILE_ASSERT(IsPointer<const char*>::value, WTF_IsPointer_const_char_star_true);
COMPILE_ASSERT(!IsPointer<char>::value, WTF_IsPointer_char_false);
COMPILE_ASSERT(!IsPointer<int>::value, WTF_IsPointer_int_false);
COMPILE_ASSERT(!IsPointer<long>::value, WTF_IsPointer_long_false);

enum TestEnum { TestEnumValue };
COMPILE_ASSERT(IsEnum<TestEnum>::value, WTF_IsEnum_enum_true);
COMPILE_ASSERT(!IsEnum<int>::value, WTF_IsEnum_int_false);
COMPILE_ASSERT(!IsEnum<TestEnum*>::value, WTF_IsEnum_enum_star_false);

COMPILE_ASSERT(IsScalar<TestEnum>::value, WTF_IsScalar_enum_true);
COMPILE_ASSERT(IsScalar<int>::value, WTF_IsScalar_int_true);
COMPILE_ASSERT(IsScalar<TestEnum*>::value, WTF_IsScalar_enum_star_true);
COMPILE_ASSERT(IsScalar<double>::value, WTF_IsScalar_double_true);

COMPILE_ASSERT(IsPod<bool>::value, WTF_IsPod_bool_true);
COMPILE_ASSERT(IsPod<char>::value, WTF_IsPod_char_true);
COMPILE_ASSERT(IsPod<signed char>::value, WTF_IsPod_signed_char_true);
Expand All @@ -90,73 +73,7 @@ COMPILE_ASSERT(IsPod<volatile char*>::value, WTF_IsPod_volatile_char_pointer_tru
COMPILE_ASSERT(IsPod<double>::value, WTF_IsPod_double_true);
COMPILE_ASSERT(IsPod<long double>::value, WTF_IsPod_long_double_true);
COMPILE_ASSERT(IsPod<float>::value, WTF_IsPod_float_true);
struct VirtualClass {
virtual void A() { }
};
COMPILE_ASSERT(!IsTriviallyMoveAssignable<VirtualClass>::value, WTF_IsTriviallyMoveAssignable_VirtualClass_false);
COMPILE_ASSERT(!IsScalar<VirtualClass>::value, WTF_IsScalar_class_false);
COMPILE_ASSERT(IsScalar<VirtualClass*>::value, WTF_IsScalar_class_ptr_true);

struct DestructorClass {
~DestructorClass() { }
};
COMPILE_ASSERT(IsTriviallyMoveAssignable<DestructorClass>::value, WTF_IsTriviallyMoveAssignable_DestructorClass_true);
COMPILE_ASSERT(IsTriviallyCopyAssignable<DestructorClass>::value, WTF_IsTriviallyCopyAssignable_DestructorClass_true);
COMPILE_ASSERT(IsTriviallyDefaultConstructible<DestructorClass>::value, WTF_IsTriviallyDefaultConstructable_DestructorClass_true);

struct MixedPrivate {
int M2() { return m2; }
int m1;
private:
int m2;
};
COMPILE_ASSERT(IsTriviallyMoveAssignable<MixedPrivate>::value, WTF_IsTriviallyMoveAssignable_MixedPrivate_true);
COMPILE_ASSERT(IsTriviallyCopyAssignable<MixedPrivate>::value, WTF_IsTriviallyCopyAssignable_MixedPrivate_true);
COMPILE_ASSERT(IsTriviallyDefaultConstructible<MixedPrivate>::value, WTF_IsTriviallyDefaultConstructable_MixedPrivate_true);
struct JustPrivate {
int M2() { return m2; }
private:
int m2;
};
COMPILE_ASSERT(IsTriviallyMoveAssignable<JustPrivate>::value, WTF_IsTriviallyMoveAssignable_JustPrivate_true);
COMPILE_ASSERT(IsTriviallyCopyAssignable<JustPrivate>::value, WTF_IsTriviallyCopyAssignable_JustPrivate_true);
COMPILE_ASSERT(IsTriviallyDefaultConstructible<JustPrivate>::value, WTF_IsTriviallyDefaultConstructable_JustPrivate_true);
struct JustPublic {
int m2;
};
COMPILE_ASSERT(IsTriviallyMoveAssignable<JustPublic>::value, WTF_IsTriviallyMoveAssignable_JustPublic_true);
COMPILE_ASSERT(IsTriviallyCopyAssignable<JustPublic>::value, WTF_IsTriviallyCopyAssignable_JustPublic_true);
COMPILE_ASSERT(IsTriviallyDefaultConstructible<JustPublic>::value, WTF_IsTriviallyDefaultConstructable_JustPublic_true);
struct NestedInherited : public JustPublic, JustPrivate {
float m3;
};
COMPILE_ASSERT(IsTriviallyMoveAssignable<NestedInherited>::value, WTF_IsTriviallyMoveAssignable_NestedInherited_true);
COMPILE_ASSERT(IsTriviallyCopyAssignable<NestedInherited>::value, WTF_IsTriviallyCopyAssignable_NestedInherited_true);
COMPILE_ASSERT(IsTriviallyDefaultConstructible<NestedInherited>::value, WTF_IsTriviallyDefaultConstructable_NestedInherited_true);
struct NestedOwned {
JustPublic m1;
JustPrivate m2;
float m3;
};

COMPILE_ASSERT(IsTriviallyMoveAssignable<NestedOwned>::value, WTF_IsTriviallyMoveAssignable_NestedOwned_true);
COMPILE_ASSERT(IsTriviallyCopyAssignable<NestedOwned>::value, WTF_IsTriviallyCopyAssignable_NestedOwned_true);
COMPILE_ASSERT(IsTriviallyDefaultConstructible<NestedOwned>::value, WTF_IsTriviallyDefaultConstructable_NestedOwned_true);

class NonCopyableClass {
#if COMPILER_SUPPORTS(CXX_DELETED_FUNCTIONS)
NonCopyableClass(const NonCopyableClass&) = delete;
NonCopyableClass& operator=(const NonCopyableClass&) = delete;
#else
NonCopyableClass(const NonCopyableClass&);
NonCopyableClass& operator=(const NonCopyableClass&);
#endif // COMPILER_SUPPORTS(CXX_DELETED_FUNCTIONS)
};
#if 0 // Compilers don't get this "right" yet if using = delete.
COMPILE_ASSERT(!IsTriviallyMoveAssignable<NonCopyableClass>::value, WTF_IsTriviallyMoveAssignable_NonCopyableClass_false);
COMPILE_ASSERT(!IsTriviallyCopyAssignable<NonCopyableClass>::value, WTF_IsTriviallyCopyAssignable_NonCopyableClass_false);
COMPILE_ASSERT(IsTriviallyDefaultConstructible<NonCopyableClass>::value, WTF_IsTriviallyDefaultConstructable_NonCopyableClass_true);
#endif // 0
COMPILE_ASSERT(!IsPod<IsPod<bool> >::value, WTF_IsPod_struct_false);

enum IsConvertibleToIntegerCheck { };
COMPILE_ASSERT(IsConvertibleToInteger<IsConvertibleToIntegerCheck>::value, WTF_IsConvertibleToInteger_enum_true);
Expand Down
45 changes: 5 additions & 40 deletions Source/wtf/TypeTraits.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ namespace WTF {
// The following are provided in this file:
//
// IsInteger<T>::value
// IsPod<T>::value
// IsPod<T>::value, see the definition for a note about its limitations
// IsConvertibleToInteger<T>::value
//
// IsArray<T>::value
Expand Down Expand Up @@ -72,52 +72,17 @@ namespace WTF {

template<typename T> struct IsArithmetic { static const bool value = IsInteger<T>::value || IsFloatingPoint<T>::value; };

template<typename T> struct IsPointer {
static const bool value = false;
};

template<typename P> struct IsPointer<const P*> {
static const bool value = true;
};

template<typename P> struct IsPointer<P*> {
static const bool value = true;
};

template<typename T> struct IsEnum {
static const bool value = __is_enum(T);
};

template<typename T> struct IsScalar {
static const bool value = IsEnum<T>::value || IsArithmetic<T>::value || IsPointer<T>::value;
};

template<typename T> struct IsWeak { static const bool value = false; };

enum WeakHandlingFlag {
NoWeakHandlingInCollections,
WeakHandlingInCollections
};

template <typename T> struct IsPod {
static const bool value = __is_pod(T);
};

template <typename T> struct IsTriviallyCopyAssignable {
static const bool value = __has_trivial_assign(T);
};

template <typename T> struct IsTriviallyMoveAssignable {
static const bool value = __has_trivial_assign(T);
};

template <typename T> struct IsTriviallyDefaultConstructible {
static const bool value = __has_trivial_constructor(T);
};

template <typename T> struct IsTriviallyDestructible {
static const bool value = __has_trivial_destructor(T);
};
// IsPod is misnamed as it doesn't cover all plain old data (pod) types.
// Specifically, it doesn't allow for enums or for structs.
template <typename T> struct IsPod { static const bool value = IsArithmetic<T>::value; };
template <typename P> struct IsPod<P*> { static const bool value = true; };

template<typename T> class IsConvertibleToInteger {
// Avoid "possible loss of data" warning when using Microsoft's C++ compiler
Expand Down
28 changes: 7 additions & 21 deletions Source/wtf/VectorTraits.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,12 @@ namespace WTF {
template<typename T>
struct VectorTraitsBase
{
static const bool needsDestruction = !IsTriviallyDestructible<T>::value;
static const bool canInitializeWithMemset = IsTriviallyDefaultConstructible<T>::value;
static const bool canMoveWithMemcpy = IsTriviallyMoveAssignable<T>::value;
static const bool canCopyWithMemcpy = IsTriviallyCopyAssignable<T>::value;
static const bool canFillWithMemset = IsTriviallyDefaultConstructible<T>::value && (sizeof(T) == sizeof(char));
static const bool canCompareWithMemcmp = IsScalar<T>::value; // Types without padding.
static const bool needsDestruction = !IsPod<T>::value;
static const bool canInitializeWithMemset = IsPod<T>::value;
static const bool canMoveWithMemcpy = IsPod<T>::value;
static const bool canCopyWithMemcpy = IsPod<T>::value;
static const bool canFillWithMemset = IsPod<T>::value && (sizeof(T) == sizeof(char));
static const bool canCompareWithMemcmp = IsPod<T>::value;
template<typename U = void>
struct NeedsTracingLazily {
static const bool value = NeedsTracing<T>::value;
Expand Down Expand Up @@ -69,18 +69,7 @@ namespace WTF {
struct VectorTraits<RefPtr<P> > : SimpleClassVectorTraits<RefPtr<P> > { };

template<typename P>
struct VectorTraits<OwnPtr<P> > : SimpleClassVectorTraits<OwnPtr<P> > {
// OwnPtr -> PassOwnPtr has a very particular structure that
// tricks the normal type traits into thinking that the class
// is "trivially copyable".
static const bool canCopyWithMemcpy = false;
};
COMPILE_ASSERT(VectorTraits<RefPtr<int> >::canInitializeWithMemset, inefficientRefPtrVector);
COMPILE_ASSERT(VectorTraits<RefPtr<int> >::canMoveWithMemcpy, inefficientRefPtrVector);
COMPILE_ASSERT(VectorTraits<RefPtr<int> >::canCompareWithMemcmp, inefficientRefPtrVector);
COMPILE_ASSERT(VectorTraits<OwnPtr<int> >::canInitializeWithMemset, inefficientOwnPtrVector);
COMPILE_ASSERT(VectorTraits<OwnPtr<int> >::canMoveWithMemcpy, inefficientOwnPtrVector);
COMPILE_ASSERT(VectorTraits<OwnPtr<int> >::canCompareWithMemcmp, inefficientOwnPtrVector);
struct VectorTraits<OwnPtr<P> > : SimpleClassVectorTraits<OwnPtr<P> > { };

template<typename First, typename Second>
struct VectorTraits<pair<First, Second> >
Expand All @@ -105,14 +94,12 @@ namespace WTF {

#define WTF_ALLOW_MOVE_INIT_AND_COMPARE_WITH_MEM_FUNCTIONS(ClassName) \
namespace WTF { \
COMPILE_ASSERT(!IsTriviallyDefaultConstructible<ClassName>::value || !IsTriviallyCopyAssignable<ClassName>::value, macro_not_needed); \
template<> \
struct VectorTraits<ClassName> : SimpleClassVectorTraits<ClassName> { }; \
}

#define WTF_ALLOW_MOVE_AND_INIT_WITH_MEM_FUNCTIONS(ClassName) \
namespace WTF { \
COMPILE_ASSERT(!WTF::IsTriviallyDefaultConstructible<ClassName>::value || !IsTriviallyCopyAssignable<ClassName>::value, macro_not_needed); \
template<> \
struct VectorTraits<ClassName> : VectorTraitsBase<ClassName> \
{ \
Expand All @@ -123,7 +110,6 @@ COMPILE_ASSERT(!WTF::IsTriviallyDefaultConstructible<ClassName>::value || !IsTri

#define WTF_ALLOW_INIT_WITH_MEM_FUNCTIONS(ClassName) \
namespace WTF { \
COMPILE_ASSERT(!WTF::IsTriviallyDefaultConstructible<ClassName>::value, macro_not_needed); \
template<> \
struct VectorTraits<ClassName> : VectorTraitsBase<ClassName> \
{ \
Expand Down

0 comments on commit 463f3d2

Please sign in to comment.