Skip to content

Commit 69f9fd4

Browse files
sherginfacebook-github-bot
authored andcommitted
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
1 parent bfc9839 commit 69f9fd4

File tree

2 files changed

+8
-27
lines changed

2 files changed

+8
-27
lines changed

ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.cpp

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,6 @@
1212
namespace facebook {
1313
namespace react {
1414

15-
ImageResponseObserverCoordinator::ImageResponseObserverCoordinator() {
16-
status_ = ImageResponse::Status::Loading;
17-
}
18-
19-
ImageResponseObserverCoordinator::~ImageResponseObserverCoordinator() {}
20-
2115
void ImageResponseObserverCoordinator::addObserver(
2216
ImageResponseObserver *observer) const {
2317
ImageResponse::Status status = [this] {
@@ -51,7 +45,7 @@ void ImageResponseObserverCoordinator::removeObserver(
5145

5246
void ImageResponseObserverCoordinator::nativeImageResponseProgress(
5347
float progress) const {
54-
std::vector<ImageResponseObserver *> observersCopy = [this] {
48+
auto observersCopy = [this] {
5549
std::shared_lock<better::shared_mutex> read(mutex_);
5650
return observers_;
5751
}();
@@ -69,17 +63,13 @@ void ImageResponseObserverCoordinator::nativeImageResponseComplete(
6963
status_ = ImageResponse::Status::Completed;
7064
}
7165

72-
std::vector<ImageResponseObserver *> observersCopy = [this] {
66+
auto observersCopy = [this] {
7367
std::shared_lock<better::shared_mutex> read(mutex_);
7468
return observers_;
7569
}();
7670

7771
for (auto observer : observersCopy) {
78-
ImageResponse imageResponseCopy = [this] {
79-
std::unique_lock<better::shared_mutex> read(mutex_);
80-
return ImageResponse(imageData_);
81-
}();
82-
observer->didReceiveImage(imageResponseCopy);
72+
observer->didReceiveImage(imageResponse);
8373
}
8474
}
8575

@@ -89,7 +79,7 @@ void ImageResponseObserverCoordinator::nativeImageResponseFailed() const {
8979
status_ = ImageResponse::Status::Failed;
9080
}
9181

92-
std::vector<ImageResponseObserver *> observersCopy = [this] {
82+
auto observersCopy = [this] {
9383
std::shared_lock<better::shared_mutex> read(mutex_);
9484
return observers_;
9585
}();

ReactCommon/fabric/imagemanager/ImageResponseObserverCoordinator.h

Lines changed: 4 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <react/imagemanager/ImageResponseObserver.h>
1212

1313
#include <better/mutex.h>
14+
#include <better/small_vector.h>
1415
#include <vector>
1516

1617
namespace facebook {
@@ -24,16 +25,6 @@ namespace react {
2425
*/
2526
class ImageResponseObserverCoordinator {
2627
public:
27-
/*
28-
* Default constructor.
29-
*/
30-
ImageResponseObserverCoordinator();
31-
32-
/*
33-
* Default destructor.
34-
*/
35-
~ImageResponseObserverCoordinator();
36-
3728
/*
3829
* Interested parties may observe the image response.
3930
*/
@@ -66,19 +57,19 @@ class ImageResponseObserverCoordinator {
6657
* List of observers.
6758
* Mutable: protected by mutex_.
6859
*/
69-
mutable std::vector<ImageResponseObserver *> observers_;
60+
mutable better::small_vector<ImageResponseObserver *, 1> observers_;
7061

7162
/*
7263
* Current status of image loading.
7364
* Mutable: protected by mutex_.
7465
*/
75-
mutable ImageResponse::Status status_;
66+
mutable ImageResponse::Status status_{ImageResponse::Status::Loading};
7667

7768
/*
7869
* Cache image data.
7970
* Mutable: protected by mutex_.
8071
*/
81-
mutable std::shared_ptr<void> imageData_{};
72+
mutable std::shared_ptr<void> imageData_;
8273

8374
/*
8475
* Observer and data mutex.

0 commit comments

Comments
 (0)