From 69f9fd4f960cdfdf12fc9e30e7fd55f97cbbc2da Mon Sep 17 00:00:00 2001
From: Valentin Shergin <shergin@fb.com>
Date: Mon, 23 Sep 2019 15:30:32 -0700
Subject: [PATCH] Fabirc: Improvements in `ImageResponseObserverCoordinator`

Summary:
This diff contains some small improvements in `ImageResponseObserverCoordinator` which are pretty minor:
 * Now we use `small_vector` instead of a normal one. In the vast majority of cases, the coordinator has only one observer, so having `small_vector` with default size `1` saves us a memory allocation (which is a dozen of allocations for a screen, not bad).
 * Empty constructor and destructor were removed.
 * Unnecessary copying of ImageResponse was removed. ImageResponse is a practically a shared_pointer, it has value semantic and does not need to be copied. We don't need to acquire mutex to access that.

Reviewed By: sammy-SC

Differential Revision: D17368740

fbshipit-source-id: 828e27a72b9c8ac0063c5fbda00f83ddb255309c
---
 .../ImageResponseObserverCoordinator.cpp       | 18 ++++--------------
 .../ImageResponseObserverCoordinator.h         | 17 ++++-------------
 2 files changed, 8 insertions(+), 27 deletions(-)

diff --git a/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp b/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp
index c4b5a317957..eeee9a2222c 100644
--- a/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp
+++ b/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp
@@ -12,12 +12,6 @@
 namespace facebook {
 namespace react {
 
-ImageResponseObserverCoordinator::ImageResponseObserverCoordinator() {
-  status_ = ImageResponse::Status::Loading;
-}
-
-ImageResponseObserverCoordinator::~ImageResponseObserverCoordinator() {}
-
 void ImageResponseObserverCoordinator::addObserver(
     ImageResponseObserver *observer) const {
   ImageResponse::Status status = [this] {
@@ -51,7 +45,7 @@ void ImageResponseObserverCoordinator::removeObserver(
 
 void ImageResponseObserverCoordinator::nativeImageResponseProgress(
     float progress) const {
-  std::vector<ImageResponseObserver *> observersCopy = [this] {
+  auto observersCopy = [this] {
     std::shared_lock<better::shared_mutex> read(mutex_);
     return observers_;
   }();
@@ -69,17 +63,13 @@ void ImageResponseObserverCoordinator::nativeImageResponseComplete(
     status_ = ImageResponse::Status::Completed;
   }
 
-  std::vector<ImageResponseObserver *> observersCopy = [this] {
+  auto observersCopy = [this] {
     std::shared_lock<better::shared_mutex> read(mutex_);
     return observers_;
   }();
 
   for (auto observer : observersCopy) {
-    ImageResponse imageResponseCopy = [this] {
-      std::unique_lock<better::shared_mutex> read(mutex_);
-      return ImageResponse(imageData_);
-    }();
-    observer->didReceiveImage(imageResponseCopy);
+    observer->didReceiveImage(imageResponse);
   }
 }
 
@@ -89,7 +79,7 @@ void ImageResponseObserverCoordinator::nativeImageResponseFailed() const {
     status_ = ImageResponse::Status::Failed;
   }
 
-  std::vector<ImageResponseObserver *> observersCopy = [this] {
+  auto observersCopy = [this] {
     std::shared_lock<better::shared_mutex> read(mutex_);
     return observers_;
   }();
diff --git a/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h b/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h
index 1f0500ac6fb..3aaa3d1fa0e 100644
--- a/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h
+++ b/ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h
@@ -11,6 +11,7 @@
 #include <react/imagemanager/ImageResponseObserver.h>
 
 #include <better/mutex.h>
+#include <better/small_vector.h>
 #include <vector>
 
 namespace facebook {
@@ -24,16 +25,6 @@ namespace react {
  */
 class ImageResponseObserverCoordinator {
  public:
-  /*
-   * Default constructor.
-   */
-  ImageResponseObserverCoordinator();
-
-  /*
-   * Default destructor.
-   */
-  ~ImageResponseObserverCoordinator();
-
   /*
    * Interested parties may observe the image response.
    */
@@ -66,19 +57,19 @@ class ImageResponseObserverCoordinator {
    * List of observers.
    * Mutable: protected by mutex_.
    */
-  mutable std::vector<ImageResponseObserver *> observers_;
+  mutable better::small_vector<ImageResponseObserver *, 1> observers_;
 
   /*
    * Current status of image loading.
    * Mutable: protected by mutex_.
    */
-  mutable ImageResponse::Status status_;
+  mutable ImageResponse::Status status_{ImageResponse::Status::Loading};
 
   /*
    * Cache image data.
    * Mutable: protected by mutex_.
    */
-  mutable std::shared_ptr<void> imageData_{};
+  mutable std::shared_ptr<void> imageData_;
 
   /*
    * Observer and data mutex.