summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorSeigo Nonaka <nona@google.com>2018-02-08 16:48:33 -0800
committerSeigo Nonaka <nona@google.com>2018-02-09 13:40:12 -0800
commit7d8c4ed5b7683e16e6205a6b52b1910f8ff6b51b (patch)
treebb501c01f38f75b86b9741012a6074a33450b20c
parent48fa54111e68986bfa675cc4005486f9d336db1d (diff)
downloadminikin-7d8c4ed5b7683e16e6205a6b52b1910f8ff6b51b.tar.gz
Remove global mutex lock from minikin
All modules are now free from gMinikinLock. This increases the performance of parallel text layout in background thread. Here is a raw performance score on walleye-userdebug StaticLayout creation time in parallel: RandomText Thread 1 : 7,348,118 -> 7,327,542: (-0.3%) RandomText Thread 2 : 13,872,214 -> 7,402,398: (-46.6%) RandomText Thread 4 : 24,999,986 -> 8,185,263: (-67.3%) StaticLayout creation time: MeasuredText Balanced Hyphenation : 774,245 -> 781,534: (+0.9%) MeasuredText Balanced NoHyphenation: 587,157 -> 574,882: (-2.1%) MeasuredText Greedy Hyphenation : 535,843 -> 523,630: (-2.3%) MeasuredText Greedy NoHyphenation : 534,965 -> 522,259: (-2.4%) RandomText Balanced Hyphenation : 17,886,651 -> 17,814,108: (-0.4%) RandomText Balanced NoHyphenation : 7,385,288 -> 7,410,588: (+0.3%) RandomText Greedy Hyphenation : 7,380,350 -> 7,296,407: (-1.1%) RandomText Greedy NoHyphenation : 7,358,680 -> 7,320,040: (-0.5%) MeasuredText creation time NoStyled Hyphenation : 17,587,451 -> 17,544,350: (-0.2%) NoStyled Hyphenation WidthOnly : 17,112,258 -> 17,092,507: (-0.1%) NoStyled NoHyphenation : 7,307,064 -> 7,296,519: (-0.1%) NoStyled NoHyphenation WidthOnly : 6,880,721 -> 6,848,543: (-0.5%) Styled Hyphenation : 14,753,919 -> 14,659,049: (-0.6%) Styled Hyphenation WidthOnly : 13,767,908 -> 13,755,623: (-0.1%) Styled NoHyphenation : 14,368,118 -> 14,337,125: (-0.2%) Styled NoHyphenation WidthOnly : 13,414,942 -> 13,375,858: (-0.3%) StaticLayout draw time: MeasuredText NoStyled : 676,438 -> 678,795: (+0.3%) MeasuredText NoStyled WithoutCache : 663,041 -> 654,687: (-1.3%) MeasuredText Styled : 885,574 -> 890,281: (+0.5%) MeasuredText Styled WithoutCache : 944,259 -> 928,675: (-1.7%) RandomText NoStyled : 562,361 -> 553,651: (-1.5%) RandomText NoStyled WithoutCache : 6,678,968 -> 6,696,637: (+0.3%) RandomText Styled : 2,970,369 -> 2,933,900: (-1.2%) RandomText Styled WithoutCache : 3,408,554 -> 3,387,260: (-0.6%) Bug: 37567215 Test: minikin_tests Test: minikin_stress_tests Test: atest CtsWidgetTestCases:EditTextTest CtsWidgetTestCases:TextViewFadingEdgeTest FrameworksCoreTests:TextViewFallbackLineSpacingTest FrameworksCoreTests:TextViewTest FrameworksCoreTests:TypefaceTest CtsGraphicsTestCases:TypefaceTest CtsWidgetTestCases:TextViewTest CtsTextTestCases CtsGraphicsTestCases:PaintTest Test: bit FrameworksCoreTests:android.text. Change-Id: I6aab090be6bfa65f1205b26b77c6c65823a3d94e
-rw-r--r--include/minikin/FontCollection.h3
-rw-r--r--libs/minikin/Android.bp4
-rw-r--r--libs/minikin/FontCollection.cpp8
-rw-r--r--libs/minikin/Layout.cpp9
-rw-r--r--libs/minikin/LineBreakerUtil.h1
-rw-r--r--libs/minikin/Locale.cpp1
-rw-r--r--libs/minikin/LocaleListCache.h2
-rw-r--r--libs/minikin/MinikinInternal.cpp8
-rw-r--r--libs/minikin/MinikinInternal.h9
-rw-r--r--tests/perftests/FontCollection.cpp1
-rw-r--r--tests/stresstest/FontFamilyTest.cpp1
-rw-r--r--tests/unittest/FontCollectionItemizeTest.cpp2
-rw-r--r--tests/unittest/FontFamilyTest.cpp4
-rw-r--r--tests/unittest/FontLanguageListCacheTest.cpp2
-rw-r--r--tests/unittest/HyphenatorMapTest.cpp1
-rw-r--r--tests/unittest/LineBreakerTestHelper.h1
-rw-r--r--tests/unittest/LocaleListTest.cpp1
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));
}