Merge "Remove global mutex lock from minikin"

17 files changed, 4 insertions, 54 deletions

diff --git a/include/minikin/FontCollection.h b/include/minikin/FontCollection.h
index 288ed0d..61b4231 100644
--- a/include/minikin/FontCollection.h
+++ b/include/minikin/FontCollection.h
@@ -91,9 +91,6 @@ private:
     static uint32_t calcVariantMatchingScore(FontFamily::Variant variant,
                                              const FontFamily& fontFamily);
 
-    // static for allocating unique id's
-    static uint32_t sNextId;
-
     // unique id for this font collection (suitable for cache key)
     uint32_t mId;
 
diff --git a/libs/minikin/Android.bp b/libs/minikin/Android.bp
index 0f9d003..4dc36b7 100644
--- a/libs/minikin/Android.bp
+++ b/libs/minikin/Android.bp
@@ -75,8 +75,8 @@ cc_library {
     ],
     product_variables: {
         debuggable: {
-            // Enable race detection and assertion on eng and userdebug build.
-            cppflags: ["-DENABLE_RACE_DETECTION", "-DENABLE_ASSERTION"],
+            // Enable assertion on eng and userdebug build.
+            cppflags: ["-DENABLE_ASSERTION"],
         },
     },
     shared_libs: [
diff --git a/libs/minikin/FontCollection.cpp b/libs/minikin/FontCollection.cpp
index d9b200a..b20d779 100644
--- a/libs/minikin/FontCollection.cpp
+++ b/libs/minikin/FontCollection.cpp
@@ -42,7 +42,7 @@ static inline T max(T a, T b) {
 const uint32_t EMOJI_STYLE_VS = 0xFE0F;
 const uint32_t TEXT_STYLE_VS = 0xFE0E;
 
-uint32_t FontCollection::sNextId = 0;
+static std::atomic<uint32_t> gNextCollectionId = {0};
 
 FontCollection::FontCollection(std::shared_ptr<FontFamily>&& typeface) : mMaxChar(0) {
     std::vector<std::shared_ptr<FontFamily>> typefaces;
@@ -55,8 +55,7 @@ FontCollection::FontCollection(const vector<std::shared_ptr<FontFamily>>& typefa
 }
 
 void FontCollection::init(const vector<std::shared_ptr<FontFamily>>& typefaces) {
-    android::AutoMutex _l(gMinikinLock);
-    mId = sNextId++;
+    mId = gNextCollectionId++;
     vector<uint32_t> lastChar;
     size_t nTypefaces = typefaces.size();
     const FontStyle defaultStyle;
@@ -350,9 +349,6 @@ bool FontCollection::hasVariationSelector(uint32_t baseCodepoint,
         }
     }
 
-    // TODO: We can remove this lock by precomputing color emoji information.
-    android::AutoMutex _l(gMinikinLock);
-
     // Even if there is no cmap format 14 subtable entry for the given sequence, should return true
     // for <char, text presentation selector> case since we have special fallback rule for the
     // sequence. Note that we don't need to restrict this to already standardized variation
diff --git a/libs/minikin/Layout.cpp b/libs/minikin/Layout.cpp
index a641fae..824e3a1 100644
--- a/libs/minikin/Layout.cpp
+++ b/libs/minikin/Layout.cpp
@@ -310,8 +310,6 @@ private:
     // number of strings
     static const size_t kMaxEntries = 5000;
 
-    // To avoid dead-locking, need to acquire gMinikinLock before acquiring mMutex.
-    // TODO: Remove gMinikinLock
     std::mutex mMutex;
 };
 
@@ -626,8 +624,6 @@ BidiText::BidiText(const uint16_t* buf, size_t start, size_t count, size_t bufSi
 void Layout::doLayout(const U16StringPiece& textBuf, const Range& range, Bidi bidiFlags,
                       const MinikinPaint& paint, StartHyphenEdit startHyphen,
                       EndHyphenEdit endHyphen) {
-    android::AutoMutex _l(gMinikinLock);
-
     LayoutContext ctx(paint);
 
     const uint32_t count = range.getLength();
@@ -646,7 +642,6 @@ void Layout::doLayout(const U16StringPiece& textBuf, const Range& range, Bidi bi
 void Layout::addToLayoutPieces(const U16StringPiece& textBuf, const Range& range, Bidi bidiFlag,
                                const MinikinPaint& paint,
                                LayoutPieces* out) {
-    android::AutoMutex _l(gMinikinLock);
     LayoutContext ctx(paint);
 
     float advance = 0;
@@ -666,8 +661,6 @@ void Layout::addToLayoutPieces(const U16StringPiece& textBuf, const Range& range
 float Layout::measureText(const U16StringPiece& textBuf, const Range& range, Bidi bidiFlags,
                           const MinikinPaint& paint, StartHyphenEdit startHyphen,
                           EndHyphenEdit endHyphen, float* advances, MinikinExtent* extents) {
-    android::AutoMutex _l(gMinikinLock);
-
     LayoutContext ctx(paint);
 
     float advance = 0;
@@ -1149,12 +1142,10 @@ void Layout::getBounds(MinikinRect* bounds) const {
 }
 
 void Layout::purgeCaches() {
-    android::AutoMutex _l(gMinikinLock);
     LayoutCache::getInstance().clear();
 }
 
 void Layout::dumpMinikinStats(int fd) {
-    android::AutoMutex _l(gMinikinLock);
     LayoutCache::getInstance().dumpStats(fd);
 }
 
diff --git a/libs/minikin/LineBreakerUtil.h b/libs/minikin/LineBreakerUtil.h
index 5e51138..4c86604 100644
--- a/libs/minikin/LineBreakerUtil.h
+++ b/libs/minikin/LineBreakerUtil.h
@@ -56,7 +56,6 @@ inline bool isLineEndSpace(uint16_t c) {
 }
 
 inline Locale getEffectiveLocale(uint32_t localeListId) {
-    android::AutoMutex _l(gMinikinLock);
     const LocaleList& localeList = LocaleListCache::getById(localeListId);
     return localeList.empty() ? Locale() : localeList[0];
 }
diff --git a/libs/minikin/Locale.cpp b/libs/minikin/Locale.cpp
index aad8cfd..346e5e4 100644
--- a/libs/minikin/Locale.cpp
+++ b/libs/minikin/Locale.cpp
@@ -31,7 +31,6 @@ namespace minikin {
 constexpr uint32_t FIVE_BITS = 0x1f;
 
 uint32_t registerLocaleList(const std::string& locales) {
-    android::AutoMutex _l(gMinikinLock);
     return LocaleListCache::getId(locales);
 }
 
diff --git a/libs/minikin/LocaleListCache.h b/libs/minikin/LocaleListCache.h
index 2a007af..32b5f6d 100644
--- a/libs/minikin/LocaleListCache.h
+++ b/libs/minikin/LocaleListCache.h
@@ -63,8 +63,6 @@ private:
     // A map from the string representation of the font locale list to the ID.
     std::unordered_map<std::string, uint32_t> mLocaleListLookupTable;
 
-    // To avoid dead-locking, need to acquire gMinikinLock before acquiring mMutex.
-    // TODO: Remove gMinikinLock
     std::mutex mMutex;
 };
 
diff --git a/libs/minikin/MinikinInternal.cpp b/libs/minikin/MinikinInternal.cpp
index 0ed667d..d02f71f 100644
--- a/libs/minikin/MinikinInternal.cpp
+++ b/libs/minikin/MinikinInternal.cpp
@@ -23,14 +23,6 @@
 
 namespace minikin {
 
-android::Mutex gMinikinLock;
-
-void assertMinikinLocked() {
-#ifdef ENABLE_RACE_DETECTION
-    LOG_ALWAYS_FATAL_IF(gMinikinLock.tryLock() == 0);
-#endif
-}
-
 inline static bool isBMPVariationSelector(uint32_t codePoint) {
     return VS1 <= codePoint && codePoint <= VS16;
 }
diff --git a/libs/minikin/MinikinInternal.h b/libs/minikin/MinikinInternal.h
index 05dcb9c..d90f099 100644
--- a/libs/minikin/MinikinInternal.h
+++ b/libs/minikin/MinikinInternal.h
@@ -28,15 +28,6 @@
 
 namespace minikin {
 
-// All external Minikin interfaces are designed to be thread-safe.
-// Presently, that's implemented by through a global lock, and having
-// all external interfaces take that lock.
-
-extern android::Mutex gMinikinLock;
-
-// Aborts if gMinikinLock is not acquired. Do nothing on the release build.
-void assertMinikinLocked();
-
 #ifdef ENABLE_ASSERTION
 #define MINIKIN_ASSERT(cond, ...) LOG_ALWAYS_FATAL_IF(!(cond), __VA_ARGS__)
 #else
diff --git a/tests/perftests/FontCollection.cpp b/tests/perftests/FontCollection.cpp
index 24fc665..7d7af6d 100644
--- a/tests/perftests/FontCollection.cpp
+++ b/tests/perftests/FontCollection.cpp
@@ -93,7 +93,6 @@ static void BM_FontCollection_itemize(benchmark::State& state) {
     MinikinPaint paint(collection);
     paint.localeListId = registerLocaleList(ITEMIZE_TEST_CASES[testIndex].languageTag);
 
-    android::AutoMutex _l(gMinikinLock);
     while (state.KeepRunning()) {
         result.clear();
         collection->itemize(buffer, utf16_length, paint, &result);
diff --git a/tests/stresstest/FontFamilyTest.cpp b/tests/stresstest/FontFamilyTest.cpp
index f7d8b8e..77371cb 100644
--- a/tests/stresstest/FontFamilyTest.cpp
+++ b/tests/stresstest/FontFamilyTest.cpp
@@ -36,7 +36,6 @@ TEST_P(FontFamilyHarfBuzzCompatibilityTest, CoverageTest) {
 
     std::shared_ptr<FontFamily> family = buildFontFamily(fontPath);
 
-    android::AutoMutex _l(gMinikinLock);
     hb_font_t* hbFont = family->getFont(0)->baseFont().get();
 
     for (uint32_t codePoint = 0; codePoint < MAX_UNICODE_CODE_POINT; ++codePoint) {
diff --git a/tests/unittest/FontCollectionItemizeTest.cpp b/tests/unittest/FontCollectionItemizeTest.cpp
index c130f6d..d1ff460 100644
--- a/tests/unittest/FontCollectionItemizeTest.cpp
+++ b/tests/unittest/FontCollectionItemizeTest.cpp
@@ -66,7 +66,6 @@ void itemize(const std::shared_ptr<FontCollection>& collection, const char* str,
     MinikinPaint paint(collection);
     paint.fontStyle = style;
     paint.localeListId = localeListId;
-    android::AutoMutex _l(gMinikinLock);
     collection->itemize(buf, len, paint, result);
 }
 
@@ -96,7 +95,6 @@ const std::string& getFontPath(const FontCollection::Run& run) {
 
 // Utility function to obtain LocaleList from string.
 const LocaleList& registerAndGetLocaleList(const std::string& locale_string) {
-    android::AutoMutex _l(gMinikinLock);
     return LocaleListCache::getById(LocaleListCache::getId(locale_string));
 }
 
diff --git a/tests/unittest/FontFamilyTest.cpp b/tests/unittest/FontFamilyTest.cpp
index 402f26b..7cdc6ad 100644
--- a/tests/unittest/FontFamilyTest.cpp
+++ b/tests/unittest/FontFamilyTest.cpp
@@ -28,13 +28,11 @@
 namespace minikin {
 
 static const LocaleList& createLocaleList(const std::string& input) {
-    android::AutoMutex _l(gMinikinLock);
     uint32_t localeListId = LocaleListCache::getId(input);
     return LocaleListCache::getById(localeListId);
 }
 
 static Locale createLocale(const std::string& input) {
-    android::AutoMutex _l(gMinikinLock);
     uint32_t localeListId = LocaleListCache::getId(input);
     return LocaleListCache::getById(localeListId)[0];
 }
@@ -627,8 +625,6 @@ TEST_F(FontFamilyTest, coverageTableSelectionTest) {
     std::shared_ptr<FontFamily> unicodeEnc3Font = buildFontFamily(kUnicodeEncoding3Font);
     std::shared_ptr<FontFamily> unicodeEnc4Font = buildFontFamily(kUnicodeEncoding4Font);
 
-    android::AutoMutex _l(gMinikinLock);
-
     EXPECT_TRUE(unicodeEnc1Font->hasGlyph(0x0061, 0));
     EXPECT_TRUE(unicodeEnc3Font->hasGlyph(0x0061, 0));
     EXPECT_TRUE(unicodeEnc4Font->hasGlyph(0x0061, 0));
diff --git a/tests/unittest/FontLanguageListCacheTest.cpp b/tests/unittest/FontLanguageListCacheTest.cpp
index e15cc24..e957cfc 100644
--- a/tests/unittest/FontLanguageListCacheTest.cpp
+++ b/tests/unittest/FontLanguageListCacheTest.cpp
@@ -30,7 +30,6 @@ TEST(LocaleListCacheTest, getId) {
     EXPECT_NE(0UL, registerLocaleList("jp"));
     EXPECT_NE(0UL, registerLocaleList("en,zh-Hans"));
 
-    android::AutoMutex _l(gMinikinLock);
     EXPECT_EQ(0UL, LocaleListCache::getId(""));
 
     EXPECT_EQ(LocaleListCache::getId("en"), LocaleListCache::getId("en"));
@@ -44,7 +43,6 @@ TEST(LocaleListCacheTest, getId) {
 }
 
 TEST(LocaleListCacheTest, getById) {
-    android::AutoMutex _l(gMinikinLock);
     uint32_t enLangId = LocaleListCache::getId("en");
     uint32_t jpLangId = LocaleListCache::getId("jp");
     Locale english = LocaleListCache::getById(enLangId)[0];
diff --git a/tests/unittest/HyphenatorMapTest.cpp b/tests/unittest/HyphenatorMapTest.cpp
index fb8d536..b793fe0 100644
--- a/tests/unittest/HyphenatorMapTest.cpp
+++ b/tests/unittest/HyphenatorMapTest.cpp
@@ -133,7 +133,6 @@ protected:
     const Locale& getLocale(const std::string& localeStr) {
         // In production, we reconstruct the LocaleList from the locale list ID.
         // So, do it here too.
-        android::AutoMutex _l(gMinikinLock);
         const uint32_t id = LocaleListCache::getId(localeStr);
         const LocaleList& locales = LocaleListCache::getById(id);
         MINIKIN_ASSERT(locales.size() == 1, "The input must be a single locale");
diff --git a/tests/unittest/LineBreakerTestHelper.h b/tests/unittest/LineBreakerTestHelper.h
index 0083f43..9eb73af 100644
--- a/tests/unittest/LineBreakerTestHelper.h
+++ b/tests/unittest/LineBreakerTestHelper.h
@@ -45,7 +45,6 @@ class ConstantRun : public Run {
 public:
     ConstantRun(const Range& range, const std::string& lang, float width)
             : Run(range), mPaint(nullptr /* font collection */), mWidth(width) {
-        android::AutoMutex _l(gMinikinLock);
         mLocaleListId = LocaleListCache::getId(lang);
     }
 
diff --git a/tests/unittest/LocaleListTest.cpp b/tests/unittest/LocaleListTest.cpp
index 36d8048..8056185 100644
--- a/tests/unittest/LocaleListTest.cpp
+++ b/tests/unittest/LocaleListTest.cpp
@@ -26,7 +26,6 @@ namespace minikin {
 namespace {
 
 const LocaleList& getLocaleList(const std::string& localeStr) {
-    android::AutoMutex _l(gMinikinLock);
     return LocaleListCache::getById(LocaleListCache::getId(localeStr));
 }