From 97ee89d60507006b2988734e7fd44d9d16f99fb1 Mon Sep 17 00:00:00 2001 From: Seigo Nonaka Date: Fri, 14 Apr 2017 11:08:12 -0700 Subject: [PATCH] Reduce heap memory in minikin. This patch reduces about 73 kB memory. The original SparseBitSet could contain full 32bit integers, but all of that is not necessary for Unicode code points. By reducing the supported range to up to Unicode maximum, U+10FFFF, we can save extra memory. SparseBitSet holds 256-bit sliced pages and indices of them. Previously, we needed to hold up to 2^24-1 pages for keeping 32-bit integers. This CL limits the number of pages to 2^16-1 (65535), so that SparseBitSet only supports 24-bit integers now, but this is sufficient for keeping all Unicode code points. With this change, we can change the index integer type from uint32_t to uint16_t. Bug: 37357593 Test: minikin_tests passes Change-Id: I462cc27927752c942ac5da0bf303a5afb81b87a3 --- .../flutter/include/minikin/SparseBitSet.h | 7 ++-- .../src/flutter/libs/minikin/CmapCoverage.cpp | 11 ++++++ .../flutter/libs/minikin/MinikinInternal.h | 2 + .../src/flutter/libs/minikin/SparseBitSet.cpp | 10 +++-- .../tests/unittest/CmapCoverageTest.cpp | 38 +++++++++++++++++++ 5 files changed, 62 insertions(+), 6 deletions(-) diff --git a/engine/src/flutter/include/minikin/SparseBitSet.h b/engine/src/flutter/include/minikin/SparseBitSet.h index 91288988ac..62aece209d 100644 --- a/engine/src/flutter/include/minikin/SparseBitSet.h +++ b/engine/src/flutter/include/minikin/SparseBitSet.h @@ -68,6 +68,7 @@ public: private: void initFromRanges(const uint32_t* ranges, size_t nRanges); + static const uint32_t kMaximumCapacity = 0xFFFFFF; static const int kLogValuesPerPage = 8; static const int kPageMask = (1 << kLogValuesPerPage) - 1; static const int kLogBytesPerEl = 2; @@ -77,16 +78,16 @@ private: typedef uint32_t element; static const element kElAllOnes = ~((element)0); static const element kElFirst = ((element)1) << kElMask; - static const uint32_t noZeroPage = ~0u; + static const uint16_t noZeroPage = 0xFFFF; static uint32_t calcNumPages(const uint32_t* ranges, size_t nRanges); static int CountLeadingZeros(element x); uint32_t mMaxVal; - std::unique_ptr mIndices; + std::unique_ptr mIndices; std::unique_ptr mBitmaps; - uint32_t mZeroPageIndex; + uint16_t mZeroPageIndex; // Forbid copy and assign. SparseBitSet(const SparseBitSet&) = delete; diff --git a/engine/src/flutter/libs/minikin/CmapCoverage.cpp b/engine/src/flutter/libs/minikin/CmapCoverage.cpp index ed053da788..a953304e5b 100644 --- a/engine/src/flutter/libs/minikin/CmapCoverage.cpp +++ b/engine/src/flutter/libs/minikin/CmapCoverage.cpp @@ -25,6 +25,7 @@ using std::vector; #include #include +#include "MinikinInternal.h" namespace minikin { @@ -127,6 +128,16 @@ static bool getCoverageFormat12(vector& coverage, const uint8_t* data, android_errorWriteLog(0x534e4554, "26413177"); return false; } + + // No need to read outside of Unicode code point range. + if (start > MAX_UNICODE_CODE_POINT) { + return true; + } + if (end > MAX_UNICODE_CODE_POINT) { + // file is inclusive, vector is exclusive + addRange(coverage, start, MAX_UNICODE_CODE_POINT + 1); + return true; + } addRange(coverage, start, end + 1); // file is inclusive, vector is exclusive } return true; diff --git a/engine/src/flutter/libs/minikin/MinikinInternal.h b/engine/src/flutter/libs/minikin/MinikinInternal.h index 250f63d74b..1ed0816188 100644 --- a/engine/src/flutter/libs/minikin/MinikinInternal.h +++ b/engine/src/flutter/libs/minikin/MinikinInternal.h @@ -38,6 +38,8 @@ void assertMinikinLocked(); hb_blob_t* getFontTable(const MinikinFont* minikinFont, uint32_t tag); +constexpr uint32_t MAX_UNICODE_CODE_POINT = 0x10FFFF; + // An RAII wrapper for hb_blob_t class HbBlob { public: diff --git a/engine/src/flutter/libs/minikin/SparseBitSet.cpp b/engine/src/flutter/libs/minikin/SparseBitSet.cpp index 28ae710c18..9fad6a0cc7 100644 --- a/engine/src/flutter/libs/minikin/SparseBitSet.cpp +++ b/engine/src/flutter/libs/minikin/SparseBitSet.cpp @@ -55,8 +55,12 @@ void SparseBitSet::initFromRanges(const uint32_t* ranges, size_t nRanges) { if (nRanges == 0) { return; } - mMaxVal = ranges[nRanges * 2 - 1]; - mIndices.reset(new uint32_t[(mMaxVal + kPageMask) >> kLogValuesPerPage]); + const uint32_t maxVal = ranges[nRanges * 2 - 1]; + if (maxVal >= kMaximumCapacity) { + return; + } + mMaxVal = maxVal; + mIndices.reset(new uint16_t[(mMaxVal + kPageMask) >> kLogValuesPerPage]); uint32_t nPages = calcNumPages(ranges, nRanges); mBitmaps.reset(new element[nPages << (kLogValuesPerPage - kLogBitsPerEl)]()); mZeroPageIndex = noZeroPage; @@ -124,7 +128,7 @@ uint32_t SparseBitSet::nextSetBit(uint32_t fromIndex) const { } uint32_t maxPage = (mMaxVal + kPageMask) >> kLogValuesPerPage; for (uint32_t page = fromPage + 1; page < maxPage; page++) { - uint32_t index = mIndices[page]; + uint16_t index = mIndices[page]; if (index == mZeroPageIndex) { continue; } diff --git a/engine/src/flutter/tests/unittest/CmapCoverageTest.cpp b/engine/src/flutter/tests/unittest/CmapCoverageTest.cpp index cbcc557a34..dd61940868 100644 --- a/engine/src/flutter/tests/unittest/CmapCoverageTest.cpp +++ b/engine/src/flutter/tests/unittest/CmapCoverageTest.cpp @@ -271,6 +271,34 @@ TEST(CmapCoverageTest, SingleFormat12) { } } +TEST(CmapCoverageTest, Format12_beyondTheUnicodeLimit) { + bool has_cmap_format_14_subtable = false; + { + SCOPED_TRACE("Starting range is out of Unicode code point. Should be ignored."); + std::vector cmap = CmapBuilder::buildSingleFormat12Cmap( + 0, 0, std::vector({'a', 'a', 0x110000, 0x110000})); + + SparseBitSet coverage = CmapCoverage::getCoverage( + cmap.data(), cmap.size(), &has_cmap_format_14_subtable); + EXPECT_TRUE(coverage.get('a')); + EXPECT_FALSE(coverage.get(0x110000)); + EXPECT_FALSE(has_cmap_format_14_subtable); + } + { + SCOPED_TRACE("Ending range is out of Unicode code point. Should be ignored."); + std::vector cmap = CmapBuilder::buildSingleFormat12Cmap( + 0, 0, std::vector({'a', 'a', 0x10FF00, 0x110000})); + + SparseBitSet coverage = CmapCoverage::getCoverage( + cmap.data(), cmap.size(), &has_cmap_format_14_subtable); + EXPECT_TRUE(coverage.get('a')); + EXPECT_TRUE(coverage.get(0x10FF00)); + EXPECT_TRUE(coverage.get(0x10FFFF)); + EXPECT_FALSE(coverage.get(0x110000)); + EXPECT_FALSE(has_cmap_format_14_subtable); + } +} + TEST(CmapCoverageTest, notSupportedEncodings) { bool has_cmap_format_14_subtable = false; @@ -398,6 +426,16 @@ TEST(CmapCoverageTest, brokenFormat12Table) { builder.appendTable(0, 0, table); std::vector cmap = builder.build(); + SparseBitSet coverage = CmapCoverage::getCoverage( + cmap.data(), cmap.size(), &has_cmap_format_14_subtable); + EXPECT_EQ(0U, coverage.length()); + EXPECT_FALSE(has_cmap_format_14_subtable); + } + { + SCOPED_TRACE("Too large code point"); + std::vector cmap = CmapBuilder::buildSingleFormat12Cmap( + 0, 0, std::vector({0x110000, 0x110000})); + SparseBitSet coverage = CmapCoverage::getCoverage( cmap.data(), cmap.size(), &has_cmap_format_14_subtable); EXPECT_EQ(0U, coverage.length());