Cache DeviceDisplayInfo data in shared structure on native side to avoid frequent JNI calls.

ui/gfx/android/device_display_info.cc

Index: ui/gfx/android/device_display_info.cc
diff --git a/ui/gfx/android/device_display_info.cc b/ui/gfx/android/device_display_info.cc
index 16fd2acb1d8a280f67c787dce12ddab15645c650..f93446fc33d8bb2ee7d5e92b9be81722fb24142d 100644
--- a/ui/gfx/android/device_display_info.cc
+++ b/ui/gfx/android/device_display_info.cc
@@ -14,52 +14,139 @@ using base::android::ScopedJavaLocalRef;
 
 namespace gfx {
 
+

[Yaron, 2013/10/10 11:52:46]

nit: remove

 DeviceDisplayInfo::DeviceDisplayInfo() {
-  JNIEnv* env = AttachCurrentThread();
-  j_device_info_.Reset(Java_DeviceDisplayInfo_create(env,
-      base::android::GetApplicationContext()));
 }
 
 DeviceDisplayInfo::~DeviceDisplayInfo() {
 }
 
 int DeviceDisplayInfo::GetDisplayHeight() {
-  JNIEnv* env = AttachCurrentThread();
-  jint result =
-      Java_DeviceDisplayInfo_getDisplayHeight(env, j_device_info_.obj());
-  return static_cast<int>(result);
+  return SharedDisplayInfo::GetDisplayHeight();
 }
 
 int DeviceDisplayInfo::GetDisplayWidth() {
-  JNIEnv* env = AttachCurrentThread();
-  jint result =
-      Java_DeviceDisplayInfo_getDisplayWidth(env, j_device_info_.obj());
-  return static_cast<int>(result);
+  return SharedDisplayInfo::GetDisplayWidth();
 }
 
 int DeviceDisplayInfo::GetBitsPerPixel() {
-  JNIEnv* env = AttachCurrentThread();
-  jint result =
-      Java_DeviceDisplayInfo_getBitsPerPixel(env, j_device_info_.obj());
-  return static_cast<int>(result);
+  return SharedDisplayInfo::GetBitsPerPixel();
 }
 
 int DeviceDisplayInfo::GetBitsPerComponent() {
-  JNIEnv* env = AttachCurrentThread();
-  jint result =
-      Java_DeviceDisplayInfo_getBitsPerComponent(env, j_device_info_.obj());
-  return static_cast<int>(result);
+  return SharedDisplayInfo::GetBitsPerComponent();
 }
 
 double DeviceDisplayInfo::GetDIPScale() {
+  return SharedDisplayInfo::GetDIPScale();
+}
+
+static pthread_rwlock_t rwlock_ = PTHREAD_RWLOCK_INITIALIZER;
+
+struct ScopedReadLock {

[Yaron, 2013/10/10 11:52:46]

Use the locking primitives from base if you still need beyond LazyInstance

[ostap, 2013/10/10 16:29:50]

I'm sorry, I didn't find read/write lock implementation in base. Using simple
base::Lock would decrease reads performance.

+  ScopedReadLock() {
+    pthread_rwlock_rdlock(&rwlock_);
+  }
+
+  ~ScopedReadLock() {
+    pthread_rwlock_unlock(&rwlock_);
+  }
+};
+
+struct ScopedWriteLock {
+  ScopedWriteLock() {
+    pthread_rwlock_wrlock(&rwlock_);
+  }
+
+  ~ScopedWriteLock() {
+    pthread_rwlock_unlock(&rwlock_);
+  }
+};
+
+scoped_ptr<SharedDisplayInfo> SharedDisplayInfo::instance_;

[Yaron, 2013/10/10 11:52:46]

Use base::LazyInstance

+
+int SharedDisplayInfo::GetDisplayHeight() {
+  SharedDisplayInfo* instance = Instance();
+  ScopedReadLock read_lock;
+  return instance->display_height_;
+}
+
+int SharedDisplayInfo::GetDisplayWidth() {
+  SharedDisplayInfo* instance = Instance();
+  ScopedReadLock read_lock;
+  return instance->display_width_;
+}
+
+int SharedDisplayInfo::GetBitsPerPixel() {
+  SharedDisplayInfo* instance = Instance();
+  ScopedReadLock read_lock;
+  return instance->bits_per_pixel_;
+}
+
+int SharedDisplayInfo::GetBitsPerComponent() {
+  SharedDisplayInfo* instance = Instance();
+  ScopedReadLock read_lock;
+  return instance->bits_per_component_;
+}
+
+double SharedDisplayInfo::GetDIPScale() {
+  SharedDisplayInfo* instance = Instance();
+  ScopedReadLock read_lock;
+  return instance->dip_scale_;
+}
+
+void UpdateSharedDisplayInfo(
+                        JNIEnv* env, jobject obj, jint display_height,

[Yaron, 2013/10/10 11:52:46]

In C++, these should be one per line per style guide.

+                        jint display_width, jint bits_per_pixel,
+                        jint bits_per_component, jdouble dip_scale) {
+  SharedDisplayInfo::UpdateDisplayInfo(env, obj, display_height,
+            display_width, bits_per_pixel, bits_per_component, dip_scale);
+}
+
+void SharedDisplayInfo::UpdateDisplayInfo(JNIEnv*, jobject, jint display_height,

[Yaron, 2013/10/10 11:52:46]

Same as above

+      jint display_width, jint bits_per_pixel, jint bits_per_component,
+      jdouble dip_scale) {
+  SharedDisplayInfo* instance = Instance();
+
+  ScopedWriteLock write_lock;
+
+  instance->display_height_ = static_cast<int>(display_height);
+  instance->display_width_ = static_cast<int>(display_width);
+  instance->bits_per_pixel_ = static_cast<int>(bits_per_pixel);
+  instance->bits_per_component_ = static_cast<int>(bits_per_component);
+  instance->dip_scale_ = static_cast<int>(dip_scale);

[Yaron, 2013/10/10 11:52:46]

dip_scale_ is a double

+}
+
+SharedDisplayInfo* SharedDisplayInfo::Instance() {
+  if (instance_) return instance_.get();
+
+  {
+    // Have to scope this block to avoid lock conflict
+    // with UpdateDisplayInfo method.
+    ScopedWriteLock write_lock;
+    if (!instance_) {
+      instance_.reset(new SharedDisplayInfo());
+    }
+  }
+
+  instance_->RegisterListener();
+
+  return instance_.get();
+}
+
+SharedDisplayInfo::SharedDisplayInfo() {
+  JNIEnv* env = AttachCurrentThread();
+  j_device_info_.Reset(Java_DeviceDisplayInfo_create(env,

[Yaron, 2013/10/10 11:52:46]

It's not clear to me why you need this anymore.  LazyInstance will ensure you're
constructed once. The native c-tor can then call registerListener.

[ostap, 2013/10/10 15:34:40]

On 2013/10/10 11:52:46, Yaron wrote:

The problem that constructor is called inside the write lock and
registerListener calls back with UpdateDisplayInfo which also needs write lock.
I'll check what base primitives can do. For pthread_rwlock "Results are
undefined if the calling thread holds the read-write lock (whether a read or
write lock) at the time the call is made" .

+      base::android::GetApplicationContext()));
+}
+
+void SharedDisplayInfo::RegisterListener() {
   JNIEnv* env = AttachCurrentThread();
-  jdouble result =
-      Java_DeviceDisplayInfo_getDIPScale(env, j_device_info_.obj());
-  return static_cast<double>(result);
+
+  Java_DeviceDisplayInfo_registerListener(env, j_device_info_.obj());
 }
 
-// static
-bool DeviceDisplayInfo::RegisterDeviceDisplayInfo(JNIEnv* env) {
+bool SharedDisplayInfo::RegisterSharedDeviceDisplayInfo(JNIEnv* env) {
   return RegisterNativesImpl(env);
 }