From 5ea5979acadca23b42bdea8161421125bb5d104c Mon Sep 17 00:00:00 2001 From: prr Date: Mon, 1 Jul 2013 12:39:26 -0700 Subject: [PATCH] 8015144: Performance regression in ICU OpenType Layout library Reviewed-by: srl, jgodinez --- make/sun/font/Makefile | 2 +- makefiles/CompileNativeLibraries.gmk | 3 +- .../native/sun/font/layout/GlyphIterator.cpp | 64 ++++++++++++------- .../native/sun/font/layout/GlyphIterator.h | 10 ++- .../native/sun/font/layout/LETableReference.h | 18 +++--- .../sun/font/layout/OpenTypeUtilities.cpp | 11 ++-- 6 files changed, 68 insertions(+), 40 deletions(-) diff --git a/make/sun/font/Makefile b/make/sun/font/Makefile index 1cdd11934..49497661e 100644 --- a/make/sun/font/Makefile +++ b/make/sun/font/Makefile @@ -36,7 +36,7 @@ PRODUCT = sun CPLUSPLUSLIBRARY=true # Use higher optimization level -OPTIMIZATION_LEVEL = HIGHER +OPTIMIZATION_LEVEL = HIGHEST include $(BUILDDIR)/common/Defs.gmk diff --git a/makefiles/CompileNativeLibraries.gmk b/makefiles/CompileNativeLibraries.gmk index 4fcb6e846..0dd803cb4 100644 --- a/makefiles/CompileNativeLibraries.gmk +++ b/makefiles/CompileNativeLibraries.gmk @@ -1327,12 +1327,11 @@ else BUILD_LIBFONTMANAGER_FONTLIB:=$(FREETYPE2_LIBS) endif -LIBFONTMANAGER_OPTIMIZATION:=HIGH +LIBFONTMANAGER_OPTIMIZATION:=HIGHEST ifeq ($(OPENJDK_TARGET_OS),windows) LIBFONTMANAGER_EXCLUDE_FILES += X11FontScaler.c \ X11TextRenderer.c - LIBFONTMANAGER_OPTIMIZATION:=LOW else LIBFONTMANAGER_EXCLUDE_FILES += fontpath.c \ lcdglyph.c diff --git a/src/share/native/sun/font/layout/GlyphIterator.cpp b/src/share/native/sun/font/layout/GlyphIterator.cpp index d0cd46732..3ccf66fdc 100644 --- a/src/share/native/sun/font/layout/GlyphIterator.cpp +++ b/src/share/native/sun/font/layout/GlyphIterator.cpp @@ -66,6 +66,7 @@ GlyphIterator::GlyphIterator(LEGlyphStorage &theGlyphStorage, GlyphPositionAdjus nextLimit = -1; prevLimit = glyphCount; } + filterResetCache(); } GlyphIterator::GlyphIterator(GlyphIterator &that) @@ -84,6 +85,7 @@ GlyphIterator::GlyphIterator(GlyphIterator &that) glyphGroup = that.glyphGroup; glyphClassDefinitionTable = that.glyphClassDefinitionTable; markAttachClassDefinitionTable = that.markAttachClassDefinitionTable; + filterResetCache(); } GlyphIterator::GlyphIterator(GlyphIterator &that, FeatureMask newFeatureMask) @@ -102,6 +104,7 @@ GlyphIterator::GlyphIterator(GlyphIterator &that, FeatureMask newFeatureMask) glyphGroup = 0; glyphClassDefinitionTable = that.glyphClassDefinitionTable; markAttachClassDefinitionTable = that.markAttachClassDefinitionTable; + filterResetCache(); } GlyphIterator::GlyphIterator(GlyphIterator &that, le_uint16 newLookupFlags) @@ -120,6 +123,7 @@ GlyphIterator::GlyphIterator(GlyphIterator &that, le_uint16 newLookupFlags) glyphGroup = that.glyphGroup; glyphClassDefinitionTable = that.glyphClassDefinitionTable; markAttachClassDefinitionTable = that.markAttachClassDefinitionTable; + filterResetCache(); } GlyphIterator::~GlyphIterator() @@ -133,6 +137,7 @@ void GlyphIterator::reset(le_uint16 newLookupFlags, FeatureMask newFeatureMask) featureMask = newFeatureMask; glyphGroup = 0; lookupFlags = newLookupFlags; + filterResetCache(); } LEGlyphID *GlyphIterator::insertGlyphs(le_int32 count, LEErrorCode& success) @@ -381,53 +386,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 (!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); } - - switch (glyphClass) - { + switch (glyphClass) { case gcdNoGlyphClass: - return FALSE; + filterResult = FALSE; + break; case gcdSimpleGlyph: - return (lookupFlags & lfIgnoreBaseGlyphs) != 0; + filterResult = (lookupFlags & lfIgnoreBaseGlyphs) != 0; + break; case gcdLigatureGlyph: - return (lookupFlags & lfIgnoreLigatures) != 0; + filterResult = (lookupFlags & lfIgnoreLigatures) != 0; + break; case gcdMarkGlyph: - { if ((lookupFlags & lfIgnoreMarks) != 0) { - return TRUE; - } - + filterResult = TRUE; + } else { le_uint16 markAttachType = (lookupFlags & lfMarkAttachTypeMask) >> lfMarkAttachTypeShift; if ((markAttachType != 0) && (markAttachClassDefinitionTable.isValid())) { - return markAttachClassDefinitionTable - -> getGlyphClass(markAttachClassDefinitionTable, glyphID, success) != markAttachType; + filterResult = (markAttachClassDefinitionTable + -> getGlyphClass(markAttachClassDefinitionTable, glyphID, success) != markAttachType); + } else { + filterResult = FALSE; } - - return FALSE; } + break; case gcdComponentGlyph: - return (lookupFlags & lfIgnoreBaseGlyphs) != 0; + filterResult = ((lookupFlags & lfIgnoreBaseGlyphs) != 0); + break; default: - return FALSE; + filterResult = FALSE; + break; } + } + filterCacheValid = TRUE; + } + + return filterCache.result; } le_bool GlyphIterator::hasFeatureTag(le_bool matchGroup) const diff --git a/src/share/native/sun/font/layout/GlyphIterator.h b/src/share/native/sun/font/layout/GlyphIterator.h index d8db62a12..666b94dbc 100644 --- a/src/share/native/sun/font/layout/GlyphIterator.h +++ b/src/share/native/sun/font/layout/GlyphIterator.h @@ -98,7 +98,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); @@ -121,6 +121,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/src/share/native/sun/font/layout/LETableReference.h b/src/share/native/sun/font/layout/LETableReference.h index dab52b664..dbee61fba 100644 --- a/src/share/native/sun/font/layout/LETableReference.h +++ b/src/share/native/sun/font/layout/LETableReference.h @@ -376,7 +376,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(); @@ -384,31 +384,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/src/share/native/sun/font/layout/OpenTypeUtilities.cpp b/src/share/native/sun/font/layout/OpenTypeUtilities.cpp index f234e9484..4d810df69 100644 --- a/src/share/native/sun/font/layout/OpenTypeUtilities.cpp +++ b/src/share/native/sun/font/layout/OpenTypeUtilities.cpp @@ -79,6 +79,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(); @@ -89,17 +90,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; } @@ -107,9 +108,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); } } -- GitLab