From 10e4a851adea2d545c017629fa46c2dd9f833073 Mon Sep 17 00:00:00 2001 From: Joshua Gross Date: Thu, 31 Jan 2019 19:13:46 -0800 Subject: [PATCH] Address some paragraph measure cache follow-up nits Summary: Changing some T& to const T&, removing unnecessary shared_ptr. Reviewed By: shergin Differential Revision: D13845619 fbshipit-source-id: 2678f67f24445e3db105619a07534f5200e313fe --- .../text/paragraph/ParagraphComponentDescriptor.h | 6 +++--- .../text/paragraph/ParagraphMeasurementCache.h | 10 ++-------- .../text/paragraph/ParagraphShadowNode.cpp | 12 ++++++------ .../components/text/paragraph/ParagraphShadowNode.h | 6 ++++-- 4 files changed, 15 insertions(+), 19 deletions(-) diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphComponentDescriptor.h b/ReactCommon/fabric/components/text/paragraph/ParagraphComponentDescriptor.h index 40f9e111b..12d5eee28 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphComponentDescriptor.h +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphComponentDescriptor.h @@ -34,7 +34,7 @@ class ParagraphComponentDescriptor final // Every single `ParagraphShadowNode` will have a reference to // a shared `EvictingCacheMap`, a simple LRU cache for Paragraph // measurements. - measureCache_ = std::make_shared(); + measureCache_ = std::make_unique(); } void adopt(UnsharedShadowNode shadowNode) const override { @@ -50,7 +50,7 @@ class ParagraphComponentDescriptor final // `ParagraphShadowNode` uses this to cache the results of text rendering // measurements. - paragraphShadowNode->setMeasureCache(measureCache_); + paragraphShadowNode->setMeasureCache(measureCache_.get()); // All `ParagraphShadowNode`s must have leaf Yoga nodes with properly // setup measure function. @@ -59,7 +59,7 @@ class ParagraphComponentDescriptor final private: SharedTextLayoutManager textLayoutManager_; - SharedParagraphMeasurementCache measureCache_; + std::unique_ptr measureCache_; }; } // namespace react diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphMeasurementCache.h b/ReactCommon/fabric/components/text/paragraph/ParagraphMeasurementCache.h index 361c2aac1..6c4f2a890 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphMeasurementCache.h +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphMeasurementCache.h @@ -19,17 +19,11 @@ using ParagraphMeasurementCacheKey = std::tuple; using ParagraphMeasurementCacheValue = Size; -using ParagraphMeasurementCacheHash = std::hash; - -class ParagraphMeasurementCache; -using SharedParagraphMeasurementCache = - std::shared_ptr; - class ParagraphMeasurementCache { public: ParagraphMeasurementCache() : cache_{256} {} - bool exists(ParagraphMeasurementCacheKey &key) const { + bool exists(const ParagraphMeasurementCacheKey &key) const { std::lock_guard lock(mutex_); return cache_.exists(key); } @@ -42,7 +36,7 @@ class ParagraphMeasurementCache { void set( const ParagraphMeasurementCacheKey &key, - ParagraphMeasurementCacheValue &value) const { + const ParagraphMeasurementCacheValue &value) const { std::lock_guard lock(mutex_); cache_.set(key, value); } diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp index c0382d486..c9d5270ff 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.cpp @@ -33,7 +33,7 @@ void ParagraphShadowNode::setTextLayoutManager( } void ParagraphShadowNode::setMeasureCache( - SharedParagraphMeasurementCache cache) { + const ParagraphMeasurementCache *cache) { ensureUnsealed(); measureCache_ = cache; } @@ -63,16 +63,16 @@ Size ParagraphShadowNode::measure(LayoutConstraints layoutConstraints) const { // Cache results of this function so we don't need to call measure() // repeatedly - ParagraphMeasurementCacheKey hashValue = + ParagraphMeasurementCacheKey cacheKey = std::make_tuple(attributedString, attributes, layoutConstraints); - if (measureCache_->exists(hashValue)) { - return measureCache_->get(hashValue); + if (measureCache_->exists(cacheKey)) { + return measureCache_->get(cacheKey); } - Size measuredSize = textLayoutManager_->measure( + auto measuredSize = textLayoutManager_->measure( attributedString, getProps()->paragraphAttributes, layoutConstraints); - measureCache_->set(hashValue, measuredSize); + measureCache_->set(cacheKey, measuredSize); return measuredSize; } diff --git a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h index 4917defe8..9d03af4a3 100644 --- a/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h +++ b/ReactCommon/fabric/components/text/paragraph/ParagraphShadowNode.h @@ -53,8 +53,10 @@ class ParagraphShadowNode : public ConcreteViewShadowNode< * Associates a shared LRU cache with the node. * `ParagraphShadowNode` uses this to cache the results of * text rendering measurements. + * By design, the ParagraphComponentDescriptor outlives all + * shadow nodes, so it's safe for this to be a raw pointer. */ - void setMeasureCache(SharedParagraphMeasurementCache cache); + void setMeasureCache(const ParagraphMeasurementCache *cache); #pragma mark - LayoutableShadowNode @@ -69,7 +71,7 @@ class ParagraphShadowNode : public ConcreteViewShadowNode< void updateLocalDataIfNeeded(); SharedTextLayoutManager textLayoutManager_; - SharedParagraphMeasurementCache measureCache_; + const ParagraphMeasurementCache *measureCache_; /* * Cached attributed string that represents the content of the subtree started