From: Steven R. Loomis Date: Tue, 25 Jun 2013 00:46:51 +0000 (+0000) Subject: ICU-10234 performance improvements in Layout: (1) in LETableReference, making some... X-Git-Tag: milestone-59-0-1~2819 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5438ea9c7183cef5431c9e89e5eb215873387b1e;p=icu ICU-10234 performance improvements in Layout: (1) in LETableReference, making some functions inline (minimal), (2) OpenTypeUtilities moving range checking out of the inner loop, and the big one is (3) GlyphIterator, caching the result of filterGlyph. (caching more than one had minimal or negative improvement) X-SVN-Rev: 33845 --- diff --git a/icu4c/source/layout/GlyphIterator.cpp b/icu4c/source/layout/GlyphIterator.cpp index 787cdd2ecde..72d2dd13156 100644 --- a/icu4c/source/layout/GlyphIterator.cpp +++ b/icu4c/source/layout/GlyphIterator.cpp @@ -41,6 +41,7 @@ GlyphIterator::GlyphIterator(LEGlyphStorage &theGlyphStorage, GlyphPositionAdjus nextLimit = -1; prevLimit = glyphCount; } + filterResetCache(); } GlyphIterator::GlyphIterator(GlyphIterator &that) @@ -59,6 +60,7 @@ GlyphIterator::GlyphIterator(GlyphIterator &that) glyphGroup = that.glyphGroup; glyphClassDefinitionTable = that.glyphClassDefinitionTable; markAttachClassDefinitionTable = that.markAttachClassDefinitionTable; + filterResetCache(); } GlyphIterator::GlyphIterator(GlyphIterator &that, FeatureMask newFeatureMask) @@ -77,6 +79,7 @@ GlyphIterator::GlyphIterator(GlyphIterator &that, FeatureMask newFeatureMask) glyphGroup = 0; glyphClassDefinitionTable = that.glyphClassDefinitionTable; markAttachClassDefinitionTable = that.markAttachClassDefinitionTable; + filterResetCache(); } GlyphIterator::GlyphIterator(GlyphIterator &that, le_uint16 newLookupFlags) @@ -95,6 +98,7 @@ GlyphIterator::GlyphIterator(GlyphIterator &that, le_uint16 newLookupFlags) glyphGroup = that.glyphGroup; glyphClassDefinitionTable = that.glyphClassDefinitionTable; markAttachClassDefinitionTable = that.markAttachClassDefinitionTable; + filterResetCache(); } GlyphIterator::~GlyphIterator() @@ -108,6 +112,7 @@ void GlyphIterator::reset(le_uint16 newLookupFlags, FeatureMask newFeatureMask) featureMask = newFeatureMask; glyphGroup = 0; lookupFlags = newLookupFlags; + filterResetCache(); } LEGlyphID *GlyphIterator::insertGlyphs(le_int32 count, LEErrorCode& success) @@ -356,53 +361,68 @@ void GlyphIterator::setCursiveGlyph() glyphPositionAdjustments->setCursiveGlyph(position, baselineIsLogicalEnd()); } -le_bool GlyphIterator::filterGlyph(le_uint32 index) const +void GlyphIterator::filterResetCache(void) { + filterCacheValid = FALSE; +} + +le_bool GlyphIterator::filterGlyph(le_uint32 index) { - LEErrorCode success = LE_NO_ERROR; LEGlyphID glyphID = glyphStorage[index]; - le_int32 glyphClass = gcdNoGlyphClass; - - if (LE_GET_GLYPH(glyphID) >= 0xFFFE) { - return TRUE; - } - - if (glyphClassDefinitionTable.isValid()) { - glyphClass = glyphClassDefinitionTable->getGlyphClass(glyphClassDefinitionTable, glyphID, success); - } - - switch (glyphClass) - { - case gcdNoGlyphClass: - return FALSE; - case gcdSimpleGlyph: - return (lookupFlags & lfIgnoreBaseGlyphs) != 0; - - case gcdLigatureGlyph: - return (lookupFlags & lfIgnoreLigatures) != 0; - - case gcdMarkGlyph: - { - if ((lookupFlags & lfIgnoreMarks) != 0) { - return TRUE; + if (!filterCacheValid || filterCache.id != glyphID) { + filterCache.id = glyphID; + + le_bool &filterResult = filterCache.result; // NB: Making this a reference to accept the updated value, in case + // we want more fancy cacheing in the future. + if (LE_GET_GLYPH(glyphID) >= 0xFFFE) { + filterResult = TRUE; + } else { + LEErrorCode success = LE_NO_ERROR; + le_int32 glyphClass = gcdNoGlyphClass; + if (glyphClassDefinitionTable.isValid()) { + glyphClass = glyphClassDefinitionTable->getGlyphClass(glyphClassDefinitionTable, glyphID, success); } - - le_uint16 markAttachType = (lookupFlags & lfMarkAttachTypeMask) >> lfMarkAttachTypeShift; - - if ((markAttachType != 0) && (markAttachClassDefinitionTable.isValid())) { - return markAttachClassDefinitionTable - -> getGlyphClass(markAttachClassDefinitionTable, glyphID, success) != markAttachType; + switch (glyphClass) { + case gcdNoGlyphClass: + filterResult = FALSE; + break; + + case gcdSimpleGlyph: + filterResult = (lookupFlags & lfIgnoreBaseGlyphs) != 0; + break; + + case gcdLigatureGlyph: + filterResult = (lookupFlags & lfIgnoreLigatures) != 0; + break; + + case gcdMarkGlyph: + if ((lookupFlags & lfIgnoreMarks) != 0) { + filterResult = TRUE; + } else { + le_uint16 markAttachType = (lookupFlags & lfMarkAttachTypeMask) >> lfMarkAttachTypeShift; + + if ((markAttachType != 0) && (markAttachClassDefinitionTable.isValid())) { + filterResult = (markAttachClassDefinitionTable + -> getGlyphClass(markAttachClassDefinitionTable, glyphID, success) != markAttachType); + } else { + filterResult = FALSE; + } + } + break; + + case gcdComponentGlyph: + filterResult = ((lookupFlags & lfIgnoreBaseGlyphs) != 0); + break; + + default: + filterResult = FALSE; + break; } - - return FALSE; - } - - case gcdComponentGlyph: - return (lookupFlags & lfIgnoreBaseGlyphs) != 0; - - default: - return FALSE; + } + filterCacheValid = TRUE; } + + return filterCache.result; } le_bool GlyphIterator::hasFeatureTag(le_bool matchGroup) const diff --git a/icu4c/source/layout/GlyphIterator.h b/icu4c/source/layout/GlyphIterator.h index 9fafc19641b..6531434d64a 100644 --- a/icu4c/source/layout/GlyphIterator.h +++ b/icu4c/source/layout/GlyphIterator.h @@ -73,7 +73,7 @@ public: le_int32 applyInsertions(); private: - le_bool filterGlyph(le_uint32 index) const; + le_bool filterGlyph(le_uint32 index); le_bool hasFeatureTag(le_bool matchGroup) const; le_bool nextInternal(le_uint32 delta = 1); le_bool prevInternal(le_uint32 delta = 1); @@ -96,6 +96,14 @@ private: LEReferenceTo markAttachClassDefinitionTable; GlyphIterator &operator=(const GlyphIterator &other); // forbid copying of this class + + struct { + LEGlyphID id; + le_bool result; + } filterCache; + le_bool filterCacheValid; + + void filterResetCache(void); }; U_NAMESPACE_END diff --git a/icu4c/source/layout/LETableReference.h b/icu4c/source/layout/LETableReference.h index e3188e9724d..f7eb79b2e8d 100644 --- a/icu4c/source/layout/LETableReference.h +++ b/icu4c/source/layout/LETableReference.h @@ -352,7 +352,7 @@ public: * @param success error status * @param atPtr location of reference - if NULL, will be at offset zero (i.e. downcast of parent). Otherwise must be a pointer within parent's bounds. */ - LEReferenceTo(const LETableReference &parent, LEErrorCode &success, const void* atPtr) + inline LEReferenceTo(const LETableReference &parent, LEErrorCode &success, const void* atPtr) : LETableReference(parent, parent.ptrToOffset(atPtr, success), LE_UINTPTR_MAX, success) { verifyLength(0, LETableVarSizer::getSize(), success); if(LE_FAILURE(success)) clear(); @@ -360,31 +360,31 @@ public: /** * ptr plus offset */ - LEReferenceTo(const LETableReference &parent, LEErrorCode &success, const void* atPtr, size_t offset) + inline LEReferenceTo(const LETableReference &parent, LEErrorCode &success, const void* atPtr, size_t offset) : LETableReference(parent, parent.ptrToOffset(atPtr, success)+offset, LE_UINTPTR_MAX, success) { verifyLength(0, LETableVarSizer::getSize(), success); if(LE_FAILURE(success)) clear(); } - LEReferenceTo(const LETableReference &parent, LEErrorCode &success, size_t offset) + inline LEReferenceTo(const LETableReference &parent, LEErrorCode &success, size_t offset) : LETableReference(parent, offset, LE_UINTPTR_MAX, success) { verifyLength(0, LETableVarSizer::getSize(), success); if(LE_FAILURE(success)) clear(); } - LEReferenceTo(const LETableReference &parent, LEErrorCode &success) + inline LEReferenceTo(const LETableReference &parent, LEErrorCode &success) : LETableReference(parent, 0, LE_UINTPTR_MAX, success) { verifyLength(0, LETableVarSizer::getSize(), success); if(LE_FAILURE(success)) clear(); } - LEReferenceTo(const LEFontInstance *font, LETag tableTag, LEErrorCode &success) + inline LEReferenceTo(const LEFontInstance *font, LETag tableTag, LEErrorCode &success) : LETableReference(font, tableTag, success) { verifyLength(0, LETableVarSizer::getSize(), success); if(LE_FAILURE(success)) clear(); } - LEReferenceTo(const le_uint8 *data, size_t length = LE_UINTPTR_MAX) : LETableReference(data, length) {} - LEReferenceTo(const T *data, size_t length = LE_UINTPTR_MAX) : LETableReference((const le_uint8*)data, length) {} - LEReferenceTo() : LETableReference(NULL) {} + inline LEReferenceTo(const le_uint8 *data, size_t length = LE_UINTPTR_MAX) : LETableReference(data, length) {} + inline LEReferenceTo(const T *data, size_t length = LE_UINTPTR_MAX) : LETableReference((const le_uint8*)data, length) {} + inline LEReferenceTo() : LETableReference(NULL) {} - LEReferenceTo& operator=(const T* other) { + inline LEReferenceTo& operator=(const T* other) { setRaw(other); return *this; } diff --git a/icu4c/source/layout/OpenTypeUtilities.cpp b/icu4c/source/layout/OpenTypeUtilities.cpp index bd197551940..d77c63e0dfe 100644 --- a/icu4c/source/layout/OpenTypeUtilities.cpp +++ b/icu4c/source/layout/OpenTypeUtilities.cpp @@ -54,6 +54,7 @@ le_int8 OpenTypeUtilities::highBit(le_int32 value) Offset OpenTypeUtilities::getTagOffset(LETag tag, const LEReferenceToArrayOf &records, LEErrorCode &success) { + const TagAndOffsetRecord *r0 = (const TagAndOffsetRecord*)records.getAlias(); if(LE_FAILURE(success)) return 0; le_uint32 recordCount = records.getCount(); @@ -64,17 +65,17 @@ Offset OpenTypeUtilities::getTagOffset(LETag tag, const LEReferenceToArrayOftag; + const ATag &aTag = (r0+extra)->tag; if (SWAPT(aTag) <= tag) { index = extra; } } - while (probe > (1 << 0) && LE_SUCCESS(success)) { + while (probe > (1 << 0)) { probe >>= 1; { - const ATag &aTag = records.getAlias(index+probe,success)->tag; + const ATag &aTag = (r0+index+probe)->tag; if (SWAPT(aTag) <= tag) { index += probe; } @@ -82,9 +83,9 @@ Offset OpenTypeUtilities::getTagOffset(LETag tag, const LEReferenceToArrayOftag; + const ATag &aTag = (r0+index)->tag; if (SWAPT(aTag) == tag) { - return SWAPW(records.getAlias(index,success)->offset); + return SWAPW((r0+index)->offset); } }