- Revision
- 238504
- Author
- [email protected]
- Date
- 2018-11-26 11:29:23 -0800 (Mon, 26 Nov 2018)
Log Message
Streamline ListHashSet use in floating object code
https://bugs.webkit.org/show_bug.cgi?id=191957
Patch by Sam Weinig <[email protected]> on 2018-11-26
Reviewed by Alex Christensen.
Source/WebCore:
Simplify use of ListHashSet by using new raw pointer overloads and
making use of reversed order of template arguments in the find() and
contains() overloads that take hash translators.
* rendering/FloatingObjects.cpp:
(WebCore::FloatingObjects::remove):
Use raw pointer overloads of contains and remove. Remove seperate call
to find / check agains end() which is unnecessary as remove() already
does that.
* rendering/FloatingObjects.h:
(WebCore::FloatingObjectHashFunctions::hash):
(WebCore::FloatingObjectHashFunctions::equal):
(WebCore::FloatingObjectHashTranslator::hash):
(WebCore::FloatingObjectHashTranslator::equal):
Add hash()/equal() overloads for the raw pointer cases. As the FIXME
notes, this could be simplified by changing PtrHashBase to use designated
bottleneck functions for hash() and equal().
* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::containsFloat const):
(WebCore::RenderBlockFlow::insertFloatingObject):
(WebCore::RenderBlockFlow::removeFloatingObject):
(WebCore::RenderBlockFlow::hasOverhangingFloat):
(WebCore::RenderBlockFlow::addIntrudingFloats):
Use simplified calls.
* rendering/RenderBlockLineLayout.cpp:
(WebCore::RenderBlockFlow::layoutRunsAndFloatsInRange):
(WebCore::RenderBlockFlow::linkToEndLineIfNeeded):
Use simplified calls.
Source/WTF:
* wtf/ListHashSet.h:
Reverses the order of the template arguments for the find() and contains()
overload that allow specifying a hash translator to allow the compiler to
deduce type T. This simplifies call sites and matches other WTF containers.
Modified Paths
Diff
Modified: trunk/Source/WTF/ChangeLog (238503 => 238504)
--- trunk/Source/WTF/ChangeLog 2018-11-26 19:15:11 UTC (rev 238503)
+++ trunk/Source/WTF/ChangeLog 2018-11-26 19:29:23 UTC (rev 238504)
@@ -1,3 +1,15 @@
+2018-11-26 Sam Weinig <[email protected]>
+
+ Streamline ListHashSet use in floating object code
+ https://bugs.webkit.org/show_bug.cgi?id=191957
+
+ Reviewed by Alex Christensen.
+
+ * wtf/ListHashSet.h:
+ Reverses the order of the template arguments for the find() and contains()
+ overload that allow specifying a hash translator to allow the compiler to
+ deduce type T. This simplifies call sites and matches other WTF containers.
+
2018-11-25 Michael Catanzaro <[email protected]>
CRASH() should call abort() except on Darwin and in developer builds
Modified: trunk/Source/WTF/wtf/ListHashSet.h (238503 => 238504)
--- trunk/Source/WTF/wtf/ListHashSet.h 2018-11-26 19:15:11 UTC (rev 238503)
+++ trunk/Source/WTF/wtf/ListHashSet.h 2018-11-26 19:29:23 UTC (rev 238504)
@@ -112,11 +112,9 @@
// An alternate version of find() that finds the object by hashing and comparing
// with some other type, to avoid the cost of type conversion.
// The HashTranslator interface is defined in HashSet.
- // FIXME: We should reverse the order of the template arguments so that callers
- // can just pass the translator let the compiler deduce T.
- template<typename T, typename HashTranslator> iterator find(const T&);
- template<typename T, typename HashTranslator> const_iterator find(const T&) const;
- template<typename T, typename HashTranslator> bool contains(const T&) const;
+ template<typename HashTranslator, typename T> iterator find(const T&);
+ template<typename HashTranslator, typename T> const_iterator find(const T&) const;
+ template<typename HashTranslator, typename T> bool contains(const T&) const;
// The return value of add is a pair of an iterator to the new value's location,
// and a bool that is true if an new entry was added.
@@ -480,7 +478,7 @@
};
template<typename ValueType, typename U>
-template<typename T, typename HashTranslator>
+template<typename HashTranslator, typename T>
inline auto ListHashSet<ValueType, U>::find(const T& value) -> iterator
{
auto it = m_impl.template find<ListHashSetTranslatorAdapter<HashTranslator>>(value);
@@ -490,7 +488,7 @@
}
template<typename ValueType, typename U>
-template<typename T, typename HashTranslator>
+template<typename HashTranslator, typename T>
inline auto ListHashSet<ValueType, U>::find(const T& value) const -> const_iterator
{
auto it = m_impl.template find<ListHashSetTranslatorAdapter<HashTranslator>>(value);
@@ -500,7 +498,7 @@
}
template<typename ValueType, typename U>
-template<typename T, typename HashTranslator>
+template<typename HashTranslator, typename T>
inline bool ListHashSet<ValueType, U>::contains(const T& value) const
{
return m_impl.template contains<ListHashSetTranslatorAdapter<HashTranslator>>(value);
Modified: trunk/Source/WebCore/ChangeLog (238503 => 238504)
--- trunk/Source/WebCore/ChangeLog 2018-11-26 19:15:11 UTC (rev 238503)
+++ trunk/Source/WebCore/ChangeLog 2018-11-26 19:29:23 UTC (rev 238504)
@@ -1,3 +1,42 @@
+2018-11-26 Sam Weinig <[email protected]>
+
+ Streamline ListHashSet use in floating object code
+ https://bugs.webkit.org/show_bug.cgi?id=191957
+
+ Reviewed by Alex Christensen.
+
+ Simplify use of ListHashSet by using new raw pointer overloads and
+ making use of reversed order of template arguments in the find() and
+ contains() overloads that take hash translators.
+
+ * rendering/FloatingObjects.cpp:
+ (WebCore::FloatingObjects::remove):
+ Use raw pointer overloads of contains and remove. Remove seperate call
+ to find / check agains end() which is unnecessary as remove() already
+ does that.
+
+ * rendering/FloatingObjects.h:
+ (WebCore::FloatingObjectHashFunctions::hash):
+ (WebCore::FloatingObjectHashFunctions::equal):
+ (WebCore::FloatingObjectHashTranslator::hash):
+ (WebCore::FloatingObjectHashTranslator::equal):
+ Add hash()/equal() overloads for the raw pointer cases. As the FIXME
+ notes, this could be simplified by changing PtrHashBase to use designated
+ bottleneck functions for hash() and equal().
+
+ * rendering/RenderBlockFlow.cpp:
+ (WebCore::RenderBlockFlow::containsFloat const):
+ (WebCore::RenderBlockFlow::insertFloatingObject):
+ (WebCore::RenderBlockFlow::removeFloatingObject):
+ (WebCore::RenderBlockFlow::hasOverhangingFloat):
+ (WebCore::RenderBlockFlow::addIntrudingFloats):
+ Use simplified calls.
+
+ * rendering/RenderBlockLineLayout.cpp:
+ (WebCore::RenderBlockFlow::layoutRunsAndFloatsInRange):
+ (WebCore::RenderBlockFlow::linkToEndLineIfNeeded):
+ Use simplified calls.
+
2018-11-26 Jeremy Jones <[email protected]>
Use Full Screen consistently in localizable strings.
Modified: trunk/Source/WebCore/rendering/FloatingObjects.cpp (238503 => 238504)
--- trunk/Source/WebCore/rendering/FloatingObjects.cpp 2018-11-26 19:15:11 UTC (rev 238503)
+++ trunk/Source/WebCore/rendering/FloatingObjects.cpp 2018-11-26 19:29:23 UTC (rev 238504)
@@ -348,16 +348,13 @@
void FloatingObjects::remove(FloatingObject* floatingObject)
{
- ASSERT((m_set.contains<FloatingObject&, FloatingObjectHashTranslator>(*floatingObject)));
+ ASSERT((m_set.contains(floatingObject)));
decreaseObjectsCount(floatingObject->type());
ASSERT(floatingObject->isPlaced() || !floatingObject->isInPlacedTree());
if (floatingObject->isPlaced())
removePlacedObject(floatingObject);
ASSERT(!floatingObject->originatingLine());
- auto it = m_set.find<FloatingObject&, FloatingObjectHashTranslator>(*floatingObject);
- if (it == m_set.end())
- return;
- m_set.remove(it);
+ m_set.remove(floatingObject);
}
void FloatingObjects::computePlacedFloatsTree()
Modified: trunk/Source/WebCore/rendering/FloatingObjects.h (238503 => 238504)
--- trunk/Source/WebCore/rendering/FloatingObjects.h 2018-11-26 19:15:11 UTC (rev 238503)
+++ trunk/Source/WebCore/rendering/FloatingObjects.h 2018-11-26 19:29:23 UTC (rev 238504)
@@ -111,16 +111,25 @@
#endif
};
+// FIXME: This could be simplified if we made it inherit from PtrHash<std::unique_ptr<FloatingObject>> and
+// changed PtrHashBase to have all of its hash and equal functions bottleneck through single functions (as
+// is done here). That would allow us to only override those master hash and equal functions.
struct FloatingObjectHashFunctions {
- static unsigned hash(const std::unique_ptr<FloatingObject>& key) { return PtrHash<RenderBox*>::hash(&key->renderer()); }
- static bool equal(const std::unique_ptr<FloatingObject>& a, const std::unique_ptr<FloatingObject>& b) { return &a->renderer() == &b->renderer(); }
+ typedef std::unique_ptr<FloatingObject> T;
+ typedef typename WTF::GetPtrHelper<T>::PtrType PtrType;
+
+ static unsigned hash(PtrType key) { return PtrHash<RenderBox*>::hash(&key->renderer()); }
+ static bool equal(PtrType a, PtrType b) { return &a->renderer() == &b->renderer(); }
static const bool safeToCompareToEmptyOrDeleted = true;
+
+ static unsigned hash(const T& key) { return hash(WTF::getPtr(key)); }
+ static bool equal(const T& a, const T& b) { return equal(WTF::getPtr(a), WTF::getPtr(b)); }
+ static bool equal(PtrType a, const T& b) { return equal(a, WTF::getPtr(b)); }
+ static bool equal(const T& a, PtrType b) { return equal(WTF::getPtr(a), b); }
};
struct FloatingObjectHashTranslator {
static unsigned hash(const RenderBox& key) { return PtrHash<const RenderBox*>::hash(&key); }
- static unsigned hash(const FloatingObject& key) { return PtrHash<RenderBox*>::hash(&key.renderer()); }
static bool equal(const std::unique_ptr<FloatingObject>& a, const RenderBox& b) { return &a->renderer() == &b; }
- static bool equal(const std::unique_ptr<FloatingObject>& a, const FloatingObject& b) { return &a->renderer() == &b.renderer(); }
};
typedef ListHashSet<std::unique_ptr<FloatingObject>, FloatingObjectHashFunctions> FloatingObjectSet;
Modified: trunk/Source/WebCore/rendering/RenderBlockFlow.cpp (238503 => 238504)
--- trunk/Source/WebCore/rendering/RenderBlockFlow.cpp 2018-11-26 19:15:11 UTC (rev 238503)
+++ trunk/Source/WebCore/rendering/RenderBlockFlow.cpp 2018-11-26 19:29:23 UTC (rev 238504)
@@ -1985,7 +1985,7 @@
bool RenderBlockFlow::containsFloat(RenderBox& renderer) const
{
- return m_floatingObjects && m_floatingObjects->set().contains<RenderBox&, FloatingObjectHashTranslator>(renderer);
+ return m_floatingObjects && m_floatingObjects->set().contains<FloatingObjectHashTranslator>(renderer);
}
void RenderBlockFlow::styleDidChange(StyleDifference diff, const RenderStyle* oldStyle)
@@ -2233,7 +2233,7 @@
else {
// Don't insert the floatingObject again if it's already in the list
const FloatingObjectSet& floatingObjectSet = m_floatingObjects->set();
- auto it = floatingObjectSet.find<RenderBox&, FloatingObjectHashTranslator>(floatBox);
+ auto it = floatingObjectSet.find<FloatingObjectHashTranslator>(floatBox);
if (it != floatingObjectSet.end())
return it->get();
}
@@ -2267,7 +2267,7 @@
{
if (m_floatingObjects) {
const FloatingObjectSet& floatingObjectSet = m_floatingObjects->set();
- auto it = floatingObjectSet.find<RenderBox&, FloatingObjectHashTranslator>(floatBox);
+ auto it = floatingObjectSet.find<FloatingObjectHashTranslator>(floatBox);
if (it != floatingObjectSet.end()) {
auto& floatingObject = *it->get();
if (childrenInline()) {
@@ -2683,7 +2683,7 @@
return false;
const FloatingObjectSet& floatingObjectSet = m_floatingObjects->set();
- const auto it = floatingObjectSet.find<RenderBox&, FloatingObjectHashTranslator>(renderer);
+ const auto it = floatingObjectSet.find<FloatingObjectHashTranslator>(renderer);
if (it == floatingObjectSet.end())
return false;
@@ -2709,7 +2709,7 @@
for (auto prevIt = prevSet.begin(); prevIt != prevEnd; ++prevIt) {
auto& floatingObject = *prevIt->get();
if (logicalBottomForFloat(floatingObject) > logicalTopOffset) {
- if (!m_floatingObjects || !m_floatingObjects->set().contains<FloatingObject&, FloatingObjectHashTranslator>(floatingObject)) {
+ if (!m_floatingObjects || !m_floatingObjects->set().contains(&floatingObject)) {
// We create the floating object list lazily.
if (!m_floatingObjects)
createFloatingObjects();
Modified: trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp (238503 => 238504)
--- trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2018-11-26 19:15:11 UTC (rev 238503)
+++ trunk/Source/WebCore/rendering/RenderBlockLineLayout.cpp 2018-11-26 19:29:23 UTC (rev 238504)
@@ -1457,7 +1457,7 @@
auto it = floatingObjectSet.begin();
auto end = floatingObjectSet.end();
if (auto* lastFloat = layoutState.floatList().lastFloat()) {
- auto lastFloatIterator = floatingObjectSet.find<FloatingObject&, FloatingObjectHashTranslator>(*lastFloat);
+ auto lastFloatIterator = floatingObjectSet.find(lastFloat);
ASSERT(lastFloatIterator != end);
++lastFloatIterator;
it = lastFloatIterator;
@@ -1620,7 +1620,7 @@
auto it = floatingObjectSet.begin();
auto end = floatingObjectSet.end();
if (auto* lastFloat = layoutState.floatList().lastFloat()) {
- auto lastFloatIterator = floatingObjectSet.find<FloatingObject&, FloatingObjectHashTranslator>(*lastFloat);
+ auto lastFloatIterator = floatingObjectSet.find(lastFloat);
ASSERT(lastFloatIterator != end);
++lastFloatIterator;
it = lastFloatIterator;
