From 891e6dad73699d7b67861705843f95ba19ca5c4d Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Wed, 26 Feb 2020 14:57:20 -0800 Subject: [PATCH] base: add CachedProperty. Copy bionic's CachedProperty with some minor API tweaks, to allow for efficient querying of properties that rarely change. Bug: http://b/141959374 Test: treehugger Change-Id: I4dfc3f527d30262b35e871d256cec69e69f2e1d7 --- base/include/android-base/properties.h | 30 ++++++++++++++ base/properties.cpp | 57 ++++++++++++++++++++++++++ base/properties_test.cpp | 25 +++++++++++ 3 files changed, 112 insertions(+) diff --git a/base/include/android-base/properties.h b/base/include/android-base/properties.h index 31823df0a..49f1f3184 100644 --- a/base/include/android-base/properties.h +++ b/base/include/android-base/properties.h @@ -20,8 +20,11 @@ #include #include +#include #include +struct prop_info; + namespace android { namespace base { @@ -67,5 +70,32 @@ bool WaitForPropertyCreation(const std::string& key, std::chrono::milliseconds r std::chrono::milliseconds::max()); #endif +#if defined(__BIONIC__) && __cplusplus >= 201703L +// Cached system property lookup. For code that needs to read the same property multiple times, +// this class helps optimize those lookups. +class CachedProperty { + public: + explicit CachedProperty(const char* property_name); + + // Returns the current value of the underlying system property as cheaply as possible. + // The returned pointer is valid until the next call to Get. Because most callers are going + // to want to parse the string returned here and cached that as well, this function performs + // no locking, and is completely thread unsafe. It is the caller's responsibility to provide a + // lock for thread-safety. + // + // Note: *changed can be set to true even if the contents of the property remain the same. + const char* Get(bool* changed = nullptr); + + private: + std::string property_name_; + const prop_info* prop_info_; + std::optional cached_area_serial_; + std::optional cached_property_serial_; + char cached_value_[92]; + bool is_read_only_; + const char* read_only_property_; +}; +#endif + } // namespace base } // namespace android diff --git a/base/properties.cpp b/base/properties.cpp index 4731bf249..35e41a86e 100644 --- a/base/properties.cpp +++ b/base/properties.cpp @@ -30,6 +30,7 @@ #include #include +#include namespace android { namespace base { @@ -195,6 +196,62 @@ bool WaitForPropertyCreation(const std::string& key, return (WaitForPropertyCreation(key, relative_timeout, start_time) != nullptr); } +CachedProperty::CachedProperty(const char* property_name) + : property_name_(property_name), + prop_info_(nullptr), + cached_area_serial_(0), + cached_property_serial_(0), + is_read_only_(android::base::StartsWith(property_name, "ro.")), + read_only_property_(nullptr) { + static_assert(sizeof(cached_value_) == PROP_VALUE_MAX); +} + +const char* CachedProperty::Get(bool* changed) { + std::optional initial_property_serial_ = cached_property_serial_; + + // Do we have a `struct prop_info` yet? + if (prop_info_ == nullptr) { + // `__system_property_find` is expensive, so only retry if a property + // has been created since last time we checked. + uint32_t property_area_serial = __system_property_area_serial(); + if (property_area_serial != cached_area_serial_) { + prop_info_ = __system_property_find(property_name_.c_str()); + cached_area_serial_ = property_area_serial; + } + } + + if (prop_info_ != nullptr) { + // Only bother re-reading the property if it's actually changed since last time. + uint32_t property_serial = __system_property_serial(prop_info_); + if (property_serial != cached_property_serial_) { + __system_property_read_callback( + prop_info_, + [](void* data, const char*, const char* value, uint32_t serial) { + CachedProperty* instance = reinterpret_cast(data); + instance->cached_property_serial_ = serial; + // Read only properties can be larger than PROP_VALUE_MAX, but also never change value + // or location, thus we return the pointer from the shared memory directly. + if (instance->is_read_only_) { + instance->read_only_property_ = value; + } else { + strlcpy(instance->cached_value_, value, PROP_VALUE_MAX); + } + }, + this); + } + } + + if (changed) { + *changed = cached_property_serial_ != initial_property_serial_; + } + + if (is_read_only_) { + return read_only_property_; + } else { + return cached_value_; + } +} + #endif } // namespace base diff --git a/base/properties_test.cpp b/base/properties_test.cpp index e7d4880c2..c30c41e8a 100644 --- a/base/properties_test.cpp +++ b/base/properties_test.cpp @@ -230,3 +230,28 @@ TEST(properties, WaitForPropertyCreation_timeout) { GTEST_LOG_(INFO) << "This test does nothing on the host.\n"; #endif } + +TEST(properties, CachedProperty) { +#if defined(__BIONIC__) + android::base::CachedProperty cached_property("debug.libbase.CachedProperty_test"); + bool changed; + cached_property.Get(&changed); + + android::base::SetProperty("debug.libbase.CachedProperty_test", "foo"); + ASSERT_STREQ("foo", cached_property.Get(&changed)); + ASSERT_TRUE(changed); + + ASSERT_STREQ("foo", cached_property.Get(&changed)); + ASSERT_FALSE(changed); + + android::base::SetProperty("debug.libbase.CachedProperty_test", "bar"); + ASSERT_STREQ("bar", cached_property.Get(&changed)); + ASSERT_TRUE(changed); + + ASSERT_STREQ("bar", cached_property.Get(&changed)); + ASSERT_FALSE(changed); + +#else + GTEST_LOG_(INFO) << "This test does nothing on the host.\n"; +#endif +}