8015144: Performance regression in ICU OpenType Layout library

Reviewed-by: srl, jgodinez

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
 

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

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

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> 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

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<T>::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<T>::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<T>::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<T>::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<T>::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<T>& operator=(const T* other) {
+ inline LEReferenceTo<T>& operator=(const T* other) {
     setRaw(other);
     return *this;
   }

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<TagAndOffsetRecord> &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 LEReferenceToArrayOf<Tag
   le_int32 index = 0;
 
   {
-    const ATag &aTag = records.getAlias(extra,success)->tag;
+    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 LEReferenceToArrayOf<Tag
   }
 
   {
-    const ATag &aTag = records.getAlias(index,success)->tag;
+    const ATag &aTag = (r0+index)->tag;
     if (SWAPT(aTag) == tag) {
-      return SWAPW(records.getAlias(index,success)->offset);
+      return SWAPW((r0+index)->offset);
     }
   }