Reduce the cache pressure of the CSSProperty array.

CSSProperty was recently inadvertently increased from 16 to 24 bytes.
Fix that, and also get rid of the kPropertyClasses[] array; it burns
~6 kB of cache that we'd really like to have for everything else,
and is a meaningless indirection. Instead, put every single
CSS(Unresolved)Property instance into a tightly packed array,
through some union trickery, making it possible to just index directly
into it. (The vtable pointer is still an indirection, but we can't
do much about that unless we start making our own OOP system.)
This isn't strictly by the book (the casting of an union instance
to a common base class isn't blessed by the standard), but should be
fine in practice as long as we don't introduce multiple inheritance or
similar shenanigans.

Since we've now got everything in the same table (instead of having
some in a table and some in a switch/case), we can easily
get rid of all the different special cases; we no longer need
if tests for whether something is an alias, for kVariable, etc..

There are some issues with LTO inlining, necessitating a bit uglier
.h file than we'd like; I've filed a separate bug for that.

Style perftest (Zen 2, LTO but no PGO):

Initial style (µs)     Before     After    Perf      95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce                9482      9359   +1.3%  [ +0.9%,  +1.7%]
Encyclopedia            81544     81376   +0.2%  [ -0.1%,  +0.5%]
Extension              130051    129406   +0.5%  [ +0.2%,  +0.8%]
News                    38561     38224   +0.9%  [ +0.6%,  +1.3%]
Search                  11209     11116   +0.8%  [ +0.6%,  +1.1%]
Social1                 23244     23177   +0.3%  [ -0.3%,  +0.5%]
Social2                 16139     15957   +1.1%  [ +0.9%,  +1.3%]
Sports                  42543     42553   -0.0%  [ -0.2%,  +0.2%]
Video                   32494     32414   +0.2%  [ +0.0%,  +0.5%]
Geometric mean                            +0.6%  [ +0.4%,  +0.8%]

Parse (µs)             Before     After    Perf      95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce                1357      1346   +0.8%  [ +0.1%,  +1.1%]
Encyclopedia             7649      7591   +0.8%  [ +0.5%,  +1.0%]
Extension                1342      1294   +3.8%  [ +3.2%,  +5.1%]
News                     8024      7990   +0.4%  [ +0.2%,  +0.6%]
Search                   5132      5100   +0.6%  [ +0.3%,  +0.9%]
Social1                 14744     14583   +1.1%  [ +0.8%,  +1.4%]
Social2                   626       622   +0.7%  [ -0.2%,  +1.4%]
Sports                  56554     56087   +0.8%  [ +0.6%,  +1.1%]
Video                   33306     32879   +1.3%  [ +1.1%,  +1.5%]
Geometric mean                            +1.1%  [ +1.0%,  +1.3%]

Recalc style (µs)      Before     After    Perf      95% CI (BCa)
=================== ========= ========= ======= =================
ECommerce               10989     10838   +1.4%  [ +1.0%,  +1.9%]
Encyclopedia            63024     62911   +0.2%  [ -0.1%,  +0.6%]
Extension              118891    118495   +0.3%  [ +0.1%,  +0.6%]
News                    28392     28150   +0.9%  [ +0.5%,  +1.7%]
Search                   5916      5883   +0.6%  [ +0.2%,  +0.9%]
Social1                 16444     16378   +0.4%  [ -0.3%,  +0.8%]
Social2                 12838     12764   +0.6%  [ +0.4%,  +0.8%]
Sports                  21686     21723   -0.2%  [ -0.6%,  +0.1%]
Video                   21109     21078   +0.1%  [ -0.1%,  +0.7%]
Geometric mean                            +0.5%  [ +0.3%,  +0.7%]

MotionMark (win-10-perf, 95% CI):

motionmark_ramp_canvas_arcs               [ -0.3%,  +0.2%]
motionmark_ramp_canvas_lines              [ +2.6%,  +3.5%]
motionmark_ramp_design                    [ -0.3%,  +1.3%]
motionmark_ramp_images                    [ -0.8%,  +0.3%]
motionmark_ramp_leaves                    [ +0.1%,  +0.9%]
motionmark_ramp_multiply                  [ +1.4%,  +2.3%]
motionmark_ramp_paths                     [ -3.8%,  +0.9%]
motionmark_ramp_suits                     [ +0.1%,  +1.0%]

MotionMark (mac-m1_mini_2020-perf, 95% CI):

motionmark_ramp_canvas_arcs               [ +0.3%,  +0.7%]
motionmark_ramp_canvas_lines              [ -0.1%,  +0.1%]
motionmark_ramp_design                    [ -7.2%,  +0.2%]
motionmark_ramp_images                    [ -0.7%,  +2.0%]
motionmark_ramp_leaves                    [ +0.7%,  +1.2%]
motionmark_ramp_multiply                  [ -0.6%,  +1.9%]
motionmark_ramp_paths                     [ -0.2%,  +0.6%]
motionmark_ramp_suits                     [ -1.0%,  -0.6%]

Change-Id: I6150ad6398eb06d0a50f882a28f49a351ef1f940
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4573698
Reviewed-by: Anders Hartvoll Ruud <andruud@chromium.org>
Commit-Queue: Steinar H Gunderson <sesse@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1151875}

third_party/blink/renderer/build/scripts/core/css/properties/templates/css_properties.h.tmpl

@@ -67,6 +67,9 @@ namespace {{namespace}} {
 ] | reject('==', '') | join(' | ') %}
 {% set ctor_args = (not is_alias and [property_id, flags, separator] or []) %}
 // {{property.name}}
+// NOTE: Multiple inheritance is not allowed here, since the class must be
+// reinterpret_cast-able to CSSUnresolvedProperty. See css_property_instances.cc.tmpl
+// (the cast happens in GetPropertyInternal()).
 class {{class_name}} final : public {{property.superclass}} {
  public:
   constexpr {{class_name}}() : {{property.superclass}}({{ctor_args | join(', ')}}) { }

third_party/blink/renderer/build/scripts/core/css/properties/templates/css_property_instances.cc.tmpl

@@ -13,34 +13,53 @@
 #include "third_party/blink/renderer/core/css/properties/shorthands.h"
 
 namespace blink {
-namespace static_properties {
-{% for property in properties %}
-const ::blink::{{property.namespace}}::{{property.classname}} property_{{property.property_id.lower()}};
-{% endfor %}
-{% for property in aliases %}
-const ::blink::{{property.namespace}}::{{property.classname}} property_{{property.property_id.lower()}};
-{% endfor %}
-} // namespace static_properties
-
-const CSSUnresolvedProperty* GetAliasPropertyInternal(CSSPropertyID id) {
-  DCHECK_GT(id, kLastCSSProperty);  // last property id
-  DCHECK_LE(id, kLastUnresolvedCSSProperty);  // last unresolved property id
-  switch (id) {
-    {% for property in aliases %}
-    case CSSPropertyID::{{property.enum_key}}:
-      return &static_properties::property_{{property.property_id.lower()}};
-    {% endfor %}
-    default:
-      NOTREACHED();
-      return nullptr;
+
+// NOTE: Everything in here must be reinterpret_cast-able
+// to CSSUnresolvedProperty! In particular, this means that
+// multiple inheritance is forbidden. We enforce this through
+// DCHECKs as much as we can; this also checks (compile-time)
+// that everything inherits from CSSUnresolvedProperty.
+union alignas(kCSSPropertyUnionBytes) CSSPropertyUnion {
+  constexpr CSSPropertyUnion() {}  // For kInvalid.
+  constexpr CSSPropertyUnion(Variable property)
+    : variable_(std::move(property)) {
+    DCHECK(reinterpret_cast<const CSSUnresolvedProperty *>(this) ==
+        static_cast<const CSSUnresolvedProperty *>(&variable_));
+  }
+
+  {% for property in properties %}
+  constexpr CSSPropertyUnion(::blink::{{property.namespace}}::{{property.classname}} property)
+    : {{property.property_id.lower()}}_(std::move(property)) {
+    DCHECK(reinterpret_cast<const CSSUnresolvedProperty *>(this) ==
+        static_cast<const CSSUnresolvedProperty *>(&{{property.property_id.lower()}}_));
   }
-}
+  {% endfor %}
+  {% for property in aliases %}
+  constexpr CSSPropertyUnion(::blink::{{property.namespace}}::{{property.classname}} property)
+    : {{property.property_id.lower()}}_(std::move(property)) {
+    DCHECK(reinterpret_cast<const CSSUnresolvedProperty *>(this) ==
+        static_cast<const CSSUnresolvedProperty *>(&{{property.property_id.lower()}}_));
+  }
+  {% endfor %}
 
-const CSSUnresolvedProperty* const kPropertyClasses[] = {
-  nullptr,  // kInvalid.
-  nullptr,  // kVariable.
+  Variable variable_;
   {% for property in properties %}
-  &static_properties::property_{{property.property_id.lower()}},  // {{property.property_id}}
+  ::blink::{{property.namespace}}::{{property.classname}} {{property.property_id.lower()}}_;
+  {% endfor %}
+  {% for property in aliases %}
+  ::blink::{{property.namespace}}::{{property.classname}} {{property.property_id.lower()}}_;
+  {% endfor %}
+};
+static_assert(sizeof(CSSPropertyUnion) == kCSSPropertyUnionBytes);
+
+const CSSPropertyUnion kProperties[] = {
+  {},  // kInvalid.
+  Variable(),
+  {% for property in properties %}
+  ::blink::{{property.namespace}}::{{property.classname}}(),
+  {% endfor %}
+  {% for property in aliases %}
+  ::blink::{{property.namespace}}::{{property.classname}}(),
   {% endfor %}
 };
 

third_party/blink/renderer/build/scripts/core/css/properties/templates/css_property_instances.h.tmpl

@@ -23,26 +23,32 @@ namespace {{property.namespace}} { class {{property.classname}}; }
 {% for property in aliases %}
 namespace {{property.namespace}} { class {{property.classname}}; }
 {% endfor %}
-namespace static_properties {
-{% for property in properties %}
-extern CORE_EXPORT const ::blink::{{property.namespace}}::{{property.classname}} property_{{property.property_id.lower()}};
-{% endfor %}
-{% for property in aliases %}
-extern CORE_EXPORT const ::blink::{{property.namespace}}::{{property.classname}} property_{{property.property_id.lower()}};
-{% endfor %}
-} // namespace static_properties
 
-class CSSUnresolvedProperty;
-class CSSProperty;
+// We predeclare the size of the union here, so that we can inline
+// GetPropertyInternal() without #including every single CSSProperty
+// out there (which would be nearly impossible wrt. circular includes).
+// We static_assert that it's correct in the .cc file.
+// See crbug.com/1450215.
+static constexpr size_t kCSSPropertyUnionBytes = 16;
+
+union alignas(kCSSPropertyUnionBytes) CSSPropertyUnion;
 
-extern CORE_EXPORT const CSSUnresolvedProperty* const kPropertyClasses[];
+// Static instances of every single CSSProperty and CSSUnresolvedProperty,
+// indexed by CSSPropertyID.
+extern CORE_EXPORT const CSSPropertyUnion kProperties[];
 
-const CSSUnresolvedProperty* GetAliasPropertyInternal(CSSPropertyID);
+class CSSUnresolvedProperty;
+inline const CSSUnresolvedProperty* GetPropertyInternal(CSSPropertyID id) {
+  return reinterpret_cast<const CSSUnresolvedProperty *>(
+      reinterpret_cast<const char *>(kProperties) +
+          kCSSPropertyUnionBytes * static_cast<unsigned>(id));
+}
 
 {% for property in properties %}
 inline const {{property.namespace}}::{{property.classname}}&
 Get{{property.property_id}}() {
-  return ::blink::static_properties::property_{{property.property_id.lower()}};
+  return *reinterpret_cast<const {{property.namespace}}::{{property.classname}} *>(
+      GetPropertyInternal(CSSPropertyID::{{property.enum_key}}));
 }
 {% endfor %}
 

third_party/blink/renderer/core/css/css_style_declaration.cc

@@ -299,10 +299,10 @@ void CSSStyleDeclaration::NamedPropertyEnumerator(Vector<String>& names,
       }
     }
     for (CSSPropertyID property_id : kCSSPropertyAliasList) {
-      const CSSUnresolvedProperty* property_class =
-          CSSUnresolvedProperty::GetAliasProperty(property_id);
-      if (property_class->IsWebExposed(execution_context)) {
-        property_names.push_back(property_class->GetJSPropertyName());
+      const CSSUnresolvedProperty& property_class =
+          *GetPropertyInternal(property_id);
+      if (property_class.IsWebExposed(execution_context)) {
+        property_names.push_back(property_class.GetJSPropertyName());
       }
     }
     std::sort(property_names.begin(), property_names.end(),

third_party/blink/renderer/core/css/properties/css_property.cc

@@ -14,7 +14,7 @@
 namespace blink {
 
 const CSSProperty& GetCSSPropertyVariable() {
-  return To<CSSProperty>(GetCSSPropertyVariableInternal());
+  return To<CSSProperty>(*GetPropertyInternal(CSSPropertyID::kVariable));
 }
 
 bool CSSProperty::HasEqualCSSPropertyName(const CSSProperty& other) const {

third_party/blink/renderer/core/css/properties/css_property.h

@@ -30,7 +30,7 @@ class CORE_EXPORT CSSProperty : public CSSUnresolvedProperty {
   static const CSSProperty& Get(CSSPropertyID id) {
     DCHECK(id != CSSPropertyID::kInvalid);
     DCHECK(id <= kLastCSSProperty);  // last property id
-    return To<CSSProperty>(CSSUnresolvedProperty::GetNonAliasProperty(id));
+    return To<CSSProperty>(*GetPropertyInternal(id));
   }
 
   static bool IsShorthand(const CSSPropertyName&);
@@ -219,16 +219,18 @@ class CORE_EXPORT CSSProperty : public CSSUnresolvedProperty {
   };
 
  private:
-  uint16_t property_id_;
-  char repetition_separator_;
-  Flags flags_;
+  static constexpr size_t kPropertyIdBits = 16;
+  uint64_t property_id_ : kPropertyIdBits;  // NOLINT(runtime/bitfields)
+  uint64_t repetition_separator_ : 8;       // NOLINT(runtime/bitfields)
+  uint64_t flags_ : 40;                     // NOLINT(runtime/bitfields)
 
   // Make sure we have room for all valid CSSPropertyIDs.
-  // (Using a smaller type here reduces CSSProperty size from 24 to 16
+  // (Using bit fields here reduces CSSProperty size from 24 to 16
   // bytes, and we have many of them that are frequently accessed
   // during style application.)
-  static_assert(sizeof(property_id_) * 8 >= kCSSPropertyIDBitLength);
+  static_assert(kPropertyIdBits >= kCSSPropertyIDBitLength);
 };
+static_assert(sizeof(CSSProperty) <= 16);
 
 template <>
 struct DowncastTraits<CSSProperty> {

third_party/blink/renderer/core/css/properties/css_property_ref.h

@@ -64,7 +64,7 @@ class CORE_EXPORT CSSPropertyRef {
 
   const CSSUnresolvedProperty& GetUnresolvedProperty() const {
     if (IsPropertyAlias(property_id_)) {
-      return *CSSUnresolvedProperty::GetAliasProperty(property_id_);
+      return *GetPropertyInternal(property_id_);
     }
     return GetProperty();
   }

third_party/blink/renderer/core/css/properties/css_unresolved_property.cc

@@ -5,30 +5,3 @@
 #include "third_party/blink/renderer/core/css/properties/css_unresolved_property.h"
 
 #include "third_party/blink/renderer/core/css/properties/longhands/variable.h"
-
-namespace blink {
-namespace {
-
-static constexpr Variable property_csspropertyvariable;
-
-}  // namespace
-
-const CSSUnresolvedProperty* CSSUnresolvedProperty::GetAliasProperty(
-    CSSPropertyID id) {
-  return GetAliasPropertyInternal(id);
-}
-
-const CSSUnresolvedProperty& CSSUnresolvedProperty::Get(CSSPropertyID id) {
-  DCHECK_NE(id, CSSPropertyID::kInvalid);
-  DCHECK_LE(id, kLastUnresolvedCSSProperty);
-  if (id <= kLastCSSProperty) {
-    return GetNonAliasProperty(id);
-  }
-  return *GetAliasProperty(id);
-}
-
-const CSSUnresolvedProperty& GetCSSPropertyVariableInternal() {
-  return property_csspropertyvariable;
-}
-
-}  // namespace blink

third_party/blink/renderer/core/css/properties/css_unresolved_property.h

@@ -15,15 +15,17 @@
 namespace blink {
 
 class ExecutionContext;
-CORE_EXPORT const CSSUnresolvedProperty& GetCSSPropertyVariableInternal();
 
 // TODO(crbug.com/793288): audit and consider redesigning how aliases are
 // handled once more of project Ribbon is done and all use of aliases can be
 // found and (hopefully) constrained.
 class CORE_EXPORT CSSUnresolvedProperty {
  public:
-  static const CSSUnresolvedProperty& Get(CSSPropertyID);
-  static const CSSUnresolvedProperty* GetAliasProperty(CSSPropertyID);
+  static const CSSUnresolvedProperty& Get(CSSPropertyID id) {
+    DCHECK_NE(id, CSSPropertyID::kInvalid);
+    DCHECK_LE(id, kLastUnresolvedCSSProperty);
+    return *GetPropertyInternal(id);
+  }
 
   // Origin trials are taken into account only when a non-nullptr
   // ExecutionContext is provided.
@@ -60,13 +62,6 @@ class CORE_EXPORT CSSUnresolvedProperty {
   }
 
  protected:
-  static const CSSUnresolvedProperty& GetNonAliasProperty(CSSPropertyID id) {
-    if (id == CSSPropertyID::kVariable) {
-      return GetCSSPropertyVariableInternal();
-    }
-    return *kPropertyClasses[static_cast<int>(id)];
-  }
-
   constexpr CSSUnresolvedProperty() = default;
 };
 

third_party/blink/renderer/core/testing/internals.cc

@@ -957,15 +957,13 @@ uint16_t Internals::compareTreeScopePosition(
     ExceptionState& exception_state) const {
   DCHECK(node1 && node2);
   const TreeScope* tree_scope1 =
-      IsA<Document>(node1)
-          ? static_cast<const TreeScope*>(To<Document>(node1))
-          : IsA<ShadowRoot>(node1)
-                ? static_cast<const TreeScope*>(To<ShadowRoot>(node1))
-                : nullptr;
+      IsA<Document>(node1) ? static_cast<const TreeScope*>(To<Document>(node1))
+      : IsA<ShadowRoot>(node1)
+          ? static_cast<const TreeScope*>(To<ShadowRoot>(node1))
+          : nullptr;
   const TreeScope* tree_scope2 =
-      IsA<Document>(node2)
-          ? static_cast<const TreeScope*>(To<Document>(node2))
-          : IsA<ShadowRoot>(node2)
+      IsA<Document>(node2) ? static_cast<const TreeScope*>(To<Document>(node2))
+      : IsA<ShadowRoot>(node2)
           ? static_cast<const TreeScope*>(To<ShadowRoot>(node2))
           : nullptr;
   if (!tree_scope1 || !tree_scope2) {
@@ -2373,7 +2371,7 @@ AtomicString Internals::htmlNamespace() {
 
 Vector<AtomicString> Internals::htmlTags() {
   Vector<AtomicString> tags(html_names::kTagsCount);
-  std::unique_ptr<const HTMLQualifiedName* []> qualified_names =
+  std::unique_ptr<const HTMLQualifiedName*[]> qualified_names =
       html_names::GetTags();
   for (wtf_size_t i = 0; i < html_names::kTagsCount; ++i)
     tags[i] = qualified_names[i]->LocalName();
@@ -2386,7 +2384,7 @@ AtomicString Internals::svgNamespace() {
 
 Vector<AtomicString> Internals::svgTags() {
   Vector<AtomicString> tags(svg_names::kTagsCount);
-  std::unique_ptr<const SVGQualifiedName* []> qualified_names =
+  std::unique_ptr<const SVGQualifiedName*[]> qualified_names =
       svg_names::GetTags();
   for (wtf_size_t i = 0; i < svg_names::kTagsCount; ++i)
     tags[i] = qualified_names[i]->LocalName();
@@ -3685,10 +3683,9 @@ Vector<String> Internals::getCSSPropertyAliases() const {
   Vector<String> result;
   for (CSSPropertyID alias : kCSSPropertyAliasList) {
     DCHECK(IsPropertyAlias(alias));
-    const CSSUnresolvedProperty* property_class =
-        CSSUnresolvedProperty::GetAliasProperty(alias);
-    if (property_class->IsWebExposed(document_->GetExecutionContext())) {
-      result.push_back(property_class->GetPropertyNameString());
+    const CSSUnresolvedProperty& property_class = *GetPropertyInternal(alias);
+    if (property_class.IsWebExposed(document_->GetExecutionContext())) {
+      result.push_back(property_class.GetPropertyNameString());
     }
   }
   return result;