Skip to content

Commit 25f10d6

Browse files
Abbondanzofacebook-github-bot
Abbondanzo
authored and
facebook-github-bot
committed
Fold optional TextAttributes fields into a presence mask when hashing (#57984)
Summary: `hash_combine` mixes each field into the previous seed, so it forms a dependency chain the CPU cannot overlap and an unset optional still costs a full link. `hash_combine_optionals` folds a run of optionals into one presence-mask link plus the engaged values, so a further optional costs a bit in the mask rather than a link. The mask is what keeps it collision-free: skipping disengaged fields alone would make the same value in two different slots hash identically. Equality moves from `std::tie` to a short-circuit chain ordered cheapest first, with the string and vector fields last, because the dominant caller is a successful cache lookup where the keys are equal and every field has to be examined. Changelog: [Internal] Reviewed By: javache Differential Revision: D115621401
1 parent 83d0be8 commit 25f10d6

16 files changed

Lines changed: 668 additions & 12 deletions

‎packages/react-native/ReactCommon/react/renderer/attributedstring/TextAttributes.h‎

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -133,38 +133,45 @@ struct hash<facebook::react::TextAttributes> {
133133
for (const auto &effect : textAttributes.textEffects) {
134134
facebook::react::hash_combine(textEffectsHash, effect);
135135
}
136-
return facebook::react::hash_combine(
136+
// The optionals are folded into one presence mask rather than being chained individually, so
137+
// that the properties a given text run does not set stay off the hash's dependency chain.
138+
size_t seed = 0;
139+
facebook::react::hash_combine(
140+
seed,
137141
textAttributes.foregroundColor,
138142
textAttributes.backgroundColor,
139143
textAttributes.opacity,
140144
textAttributes.fontFamily,
141145
textAttributes.fontSize,
142146
textAttributes.maxFontSizeMultiplier,
143147
textAttributes.fontSizeMultiplier,
148+
textAttributes.letterSpacing,
149+
textAttributes.lineHeight,
150+
textAttributes.textShadowRadius,
151+
textAttributes.textDecorationColor,
152+
textAttributes.textShadowColor,
153+
textEffectsHash);
154+
facebook::react::hash_combine_optionals(
155+
seed,
144156
textAttributes.fontWeight,
145157
textAttributes.fontStyle,
146158
textAttributes.fontVariant,
147159
textAttributes.fontVariationSettings,
148160
textAttributes.allowFontScaling,
149-
textAttributes.letterSpacing,
150161
textAttributes.textTransform,
151-
textAttributes.lineHeight,
152162
textAttributes.alignment,
153163
textAttributes.baseWritingDirection,
154164
textAttributes.lineBreakStrategy,
155165
textAttributes.lineBreakMode,
156-
textAttributes.textDecorationColor,
157166
textAttributes.textDecorationLineType,
158167
textAttributes.textDecorationStyle,
159168
textAttributes.textShadowOffset,
160-
textAttributes.textShadowRadius,
161-
textAttributes.textShadowColor,
162169
textAttributes.isHighlighted,
163170
textAttributes.isPressable,
164171
textAttributes.layoutDirection,
165172
textAttributes.accessibilityRole,
166-
textAttributes.role,
167-
textEffectsHash);
173+
textAttributes.role);
174+
return seed;
168175
}
169176
};
170177
} // namespace std

‎packages/react-native/ReactCommon/react/renderer/attributedstring/tests/TextAttributesHashTest.cpp‎

Lines changed: 94 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,94 @@
1+
/*
2+
* Copyright (c) Meta Platforms, Inc. and affiliates.
3+
*
4+
* This source code is licensed under the MIT license found in the
5+
* LICENSE file in the root directory of this source tree.
6+
*/
7+
8+
#include <gtest/gtest.h>
9+
10+
#include <optional>
11+
12+
#include <react/renderer/attributedstring/TextAttributes.h>
13+
14+
namespace facebook::react {
15+
namespace {
16+
17+
template <typename T>
18+
void expectOptionalFieldAffectsHash(
19+
const char* fieldName,
20+
std::optional<T> TextAttributes::* field) {
21+
SCOPED_TRACE(fieldName);
22+
TextAttributes baseline;
23+
TextAttributes changed;
24+
changed.*field = T{};
25+
26+
EXPECT_NE(
27+
std::hash<TextAttributes>{}(baseline),
28+
std::hash<TextAttributes>{}(changed));
29+
}
30+
31+
} // namespace
32+
33+
TEST(TextAttributesHashTest, equalAttributesHaveEqualHashes) {
34+
TextAttributes lhs;
35+
lhs.fontWeight = FontWeight::Weight400;
36+
lhs.fontStyle = FontStyle::Italic;
37+
lhs.fontVariationSettings = "'wght' 400";
38+
lhs.allowFontScaling = true;
39+
lhs.alignment = TextAlignment::Center;
40+
lhs.textShadowOffset = Size{.width = 1, .height = 2};
41+
lhs.isPressable = true;
42+
43+
TextAttributes rhs = lhs;
44+
45+
EXPECT_EQ(lhs, rhs);
46+
EXPECT_EQ(std::hash<TextAttributes>{}(lhs), std::hash<TextAttributes>{}(rhs));
47+
}
48+
49+
TEST(TextAttributesHashTest, everyOptionalHashedFieldAffectsHash) {
50+
expectOptionalFieldAffectsHash("fontWeight", &TextAttributes::fontWeight);
51+
expectOptionalFieldAffectsHash("fontStyle", &TextAttributes::fontStyle);
52+
expectOptionalFieldAffectsHash("fontVariant", &TextAttributes::fontVariant);
53+
expectOptionalFieldAffectsHash(
54+
"fontVariationSettings", &TextAttributes::fontVariationSettings);
55+
expectOptionalFieldAffectsHash(
56+
"allowFontScaling", &TextAttributes::allowFontScaling);
57+
expectOptionalFieldAffectsHash(
58+
"textTransform", &TextAttributes::textTransform);
59+
expectOptionalFieldAffectsHash("alignment", &TextAttributes::alignment);
60+
expectOptionalFieldAffectsHash(
61+
"baseWritingDirection", &TextAttributes::baseWritingDirection);
62+
expectOptionalFieldAffectsHash(
63+
"lineBreakStrategy", &TextAttributes::lineBreakStrategy);
64+
expectOptionalFieldAffectsHash(
65+
"lineBreakMode", &TextAttributes::lineBreakMode);
66+
expectOptionalFieldAffectsHash(
67+
"textDecorationLineType", &TextAttributes::textDecorationLineType);
68+
expectOptionalFieldAffectsHash(
69+
"textDecorationStyle", &TextAttributes::textDecorationStyle);
70+
expectOptionalFieldAffectsHash(
71+
"textShadowOffset", &TextAttributes::textShadowOffset);
72+
expectOptionalFieldAffectsHash(
73+
"isHighlighted", &TextAttributes::isHighlighted);
74+
expectOptionalFieldAffectsHash("isPressable", &TextAttributes::isPressable);
75+
expectOptionalFieldAffectsHash(
76+
"layoutDirection", &TextAttributes::layoutDirection);
77+
expectOptionalFieldAffectsHash(
78+
"accessibilityRole", &TextAttributes::accessibilityRole);
79+
expectOptionalFieldAffectsHash("role", &TextAttributes::role);
80+
}
81+
82+
TEST(TextAttributesHashTest, identicalValuesInDifferentSlotsHashDifferently) {
83+
TextAttributes highlighted;
84+
highlighted.isHighlighted = true;
85+
86+
TextAttributes pressable;
87+
pressable.isPressable = true;
88+
89+
EXPECT_NE(
90+
std::hash<TextAttributes>{}(highlighted),
91+
std::hash<TextAttributes>{}(pressable));
92+
}
93+
94+
} // namespace facebook::react

‎packages/react-native/ReactCommon/react/renderer/textlayoutmanager/TextMeasureCache.h‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -147,20 +147,28 @@ inline size_t textAttributesHashLayoutWise(const TextAttributes &textAttributes)
147147
{
148148
// Taking into account the same props as
149149
// `areTextAttributesEquivalentLayoutWise` mentions.
150-
return facebook::react::hash_combine(
150+
// The optionals are folded into one presence mask rather than being chained individually, so
151+
// that the properties a given text run does not set stay off the hash's dependency chain. This
152+
// is the hot path: it runs on every measurement-cache probe.
153+
size_t seed = 0;
154+
facebook::react::hash_combine(
155+
seed,
151156
textAttributes.fontFamily,
152157
textAttributes.fontSize,
153158
textAttributes.fontSizeMultiplier,
159+
textAttributes.maxFontSizeMultiplier,
160+
textAttributes.letterSpacing,
161+
textAttributes.lineHeight);
162+
facebook::react::hash_combine_optionals(
163+
seed,
154164
textAttributes.fontWeight,
155165
textAttributes.fontStyle,
156166
textAttributes.fontVariant,
157167
textAttributes.fontVariationSettings,
158168
textAttributes.allowFontScaling,
159-
textAttributes.maxFontSizeMultiplier,
160169
textAttributes.dynamicTypeRamp,
161-
textAttributes.letterSpacing,
162-
textAttributes.lineHeight,
163170
textAttributes.alignment);
171+
return seed;
164172
}
165173

166174
inline bool areAttributedStringFragmentsEquivalentLayoutWise(

‎packages/react-native/ReactCommon/react/renderer/textlayoutmanager/tests/TextLayoutManagerTest.cpp‎

Lines changed: 117 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#include <gtest/gtest.h>
99

10+
#include <array>
1011
#include <cmath>
1112

1213
#include <react/renderer/textlayoutmanager/TextMeasureCache.h>
@@ -81,6 +82,122 @@ TEST(TextLayoutManagerTest, emptyFontVariationSettingsClearInheritedSettings) {
8182
EXPECT_TRUE(parent.fontVariationSettings->empty());
8283
}
8384

85+
TEST(TextLayoutManagerTest, everyLayoutAttributeAffectsEqualityAndHash) {
86+
TextAttributes baseline;
87+
baseline.fontFamily = "Inter";
88+
baseline.fontSize = 16;
89+
baseline.fontSizeMultiplier = 1;
90+
baseline.maxFontSizeMultiplier = 2;
91+
baseline.letterSpacing = 0.5;
92+
baseline.lineHeight = 20;
93+
baseline.fontWeight = FontWeight::Weight400;
94+
baseline.fontStyle = FontStyle::Normal;
95+
baseline.fontVariant = FontVariant::Default;
96+
baseline.fontVariationSettings = "'wght' 400";
97+
baseline.allowFontScaling = true;
98+
baseline.dynamicTypeRamp = DynamicTypeRamp::Body;
99+
baseline.alignment = TextAlignment::Natural;
100+
101+
struct TestCase {
102+
const char* name;
103+
void (*mutate)(TextAttributes&);
104+
};
105+
const std::array<TestCase, 13> testCases{{
106+
{.name = "fontFamily",
107+
.mutate =
108+
[](TextAttributes& attributes) {
109+
attributes.fontFamily = "Roboto";
110+
}},
111+
{.name = "fontSize",
112+
.mutate = [](TextAttributes& attributes) { attributes.fontSize = 17; }},
113+
{.name = "fontSizeMultiplier",
114+
.mutate =
115+
[](TextAttributes& attributes) {
116+
attributes.fontSizeMultiplier = 1.5;
117+
}},
118+
{.name = "maxFontSizeMultiplier",
119+
.mutate =
120+
[](TextAttributes& attributes) {
121+
attributes.maxFontSizeMultiplier = 3;
122+
}},
123+
{.name = "letterSpacing",
124+
.mutate =
125+
[](TextAttributes& attributes) { attributes.letterSpacing = 1; }},
126+
{.name = "lineHeight",
127+
.mutate =
128+
[](TextAttributes& attributes) { attributes.lineHeight = 24; }},
129+
{.name = "fontWeight",
130+
.mutate =
131+
[](TextAttributes& attributes) {
132+
attributes.fontWeight = FontWeight::Weight700;
133+
}},
134+
{.name = "fontStyle",
135+
.mutate =
136+
[](TextAttributes& attributes) {
137+
attributes.fontStyle = FontStyle::Italic;
138+
}},
139+
{.name = "fontVariant",
140+
.mutate =
141+
[](TextAttributes& attributes) {
142+
attributes.fontVariant = FontVariant::SmallCaps;
143+
}},
144+
{.name = "fontVariationSettings",
145+
.mutate =
146+
[](TextAttributes& attributes) {
147+
attributes.fontVariationSettings = "'wght' 700";
148+
}},
149+
{.name = "allowFontScaling",
150+
.mutate =
151+
[](TextAttributes& attributes) {
152+
attributes.allowFontScaling = false;
153+
}},
154+
{.name = "dynamicTypeRamp",
155+
.mutate =
156+
[](TextAttributes& attributes) {
157+
attributes.dynamicTypeRamp = DynamicTypeRamp::Headline;
158+
}},
159+
{.name = "alignment",
160+
.mutate =
161+
[](TextAttributes& attributes) {
162+
attributes.alignment = TextAlignment::Center;
163+
}},
164+
}};
165+
166+
for (const auto& testCase : testCases) {
167+
SCOPED_TRACE(testCase.name);
168+
auto changed = baseline;
169+
testCase.mutate(changed);
170+
171+
EXPECT_FALSE(areTextAttributesEquivalentLayoutWise(baseline, changed));
172+
EXPECT_NE(
173+
textAttributesHashLayoutWise(baseline),
174+
textAttributesHashLayoutWise(changed));
175+
}
176+
}
177+
178+
TEST(TextLayoutManagerTest, equivalentPopulatedLayoutAttributesHashEqually) {
179+
TextAttributes lhs;
180+
lhs.fontFamily = "Inter";
181+
lhs.fontSize = 16;
182+
lhs.fontSizeMultiplier = 1;
183+
lhs.maxFontSizeMultiplier = 2;
184+
lhs.letterSpacing = 0.5;
185+
lhs.lineHeight = 20;
186+
lhs.fontWeight = FontWeight::Weight400;
187+
lhs.fontStyle = FontStyle::Italic;
188+
lhs.fontVariant = FontVariant::SmallCaps;
189+
lhs.fontVariationSettings = "'wght' 400";
190+
lhs.allowFontScaling = true;
191+
lhs.dynamicTypeRamp = DynamicTypeRamp::Body;
192+
lhs.alignment = TextAlignment::Center;
193+
194+
auto rhs = lhs;
195+
196+
EXPECT_TRUE(areTextAttributesEquivalentLayoutWise(lhs, rhs));
197+
EXPECT_EQ(
198+
textAttributesHashLayoutWise(lhs), textAttributesHashLayoutWise(rhs));
199+
}
200+
84201
// Measurements are rounded to the pixel grid, so a measurement cached at one
85202
// pixel scale factor must not satisfy a lookup at another. Keys that differ
86203
// only by pointScaleFactor must compare unequal.

0 commit comments

Comments
 (0)