changeset 6792:4fc1467017ea

8015144: Performance regression in ICU OpenType Layout library Reviewed-by: srl, jgodinez
author prr
date Wed, 03 Jul 2013 14:50:03 -0700
parents 95eeca6dc691
children bcd02823f5f5
files make/sun/font/Makefile src/share/native/sun/font/layout/GlyphIterator.cpp src/share/native/sun/font/layout/GlyphIterator.h src/share/native/sun/font/layout/LETableReference.h src/share/native/sun/font/layout/OpenTypeUtilities.cpp
diffstat 5 files changed, 78 insertions(+), 45 deletions(-) [+]
line wrap: on
line diff
--- a/make/sun/font/Makefile	Wed Jul 03 17:54:38 2013 +0100
+++ b/make/sun/font/Makefile	Wed Jul 03 14:50:03 2013 -0700
@@ -36,7 +36,11 @@
 CPLUSPLUSLIBRARY=true
 
 # Use higher optimization level
-OPTIMIZATION_LEVEL = HIGHER
+ifeq ($(PLATFORM), windows)
+  OPTIMIZATION_LEVEL = HIGHEST
+else
+  OPTIMIZATION_LEVEL = HIGHER
+endif
 
 include $(BUILDDIR)/common/Defs.gmk
 
--- a/src/share/native/sun/font/layout/GlyphIterator.cpp	Wed Jul 03 17:54:38 2013 +0100
+++ b/src/share/native/sun/font/layout/GlyphIterator.cpp	Wed Jul 03 14:50:03 2013 -0700
@@ -66,6 +66,7 @@
         nextLimit = -1;
         prevLimit = glyphCount;
     }
+    filterResetCache();
 }
 
 GlyphIterator::GlyphIterator(GlyphIterator &that)
@@ -84,6 +85,7 @@
     glyphGroup  = that.glyphGroup;
     glyphClassDefinitionTable = that.glyphClassDefinitionTable;
     markAttachClassDefinitionTable = that.markAttachClassDefinitionTable;
+    filterResetCache();
 }
 
 GlyphIterator::GlyphIterator(GlyphIterator &that, FeatureMask newFeatureMask)
@@ -102,6 +104,7 @@
     glyphGroup  = 0;
     glyphClassDefinitionTable = that.glyphClassDefinitionTable;
     markAttachClassDefinitionTable = that.markAttachClassDefinitionTable;
+    filterResetCache();
 }
 
 GlyphIterator::GlyphIterator(GlyphIterator &that, le_uint16 newLookupFlags)
@@ -120,6 +123,7 @@
     glyphGroup  = that.glyphGroup;
     glyphClassDefinitionTable = that.glyphClassDefinitionTable;
     markAttachClassDefinitionTable = that.markAttachClassDefinitionTable;
+    filterResetCache();
 }
 
 GlyphIterator::~GlyphIterator()
@@ -133,6 +137,7 @@
     featureMask  = newFeatureMask;
     glyphGroup   = 0;
     lookupFlags  = newLookupFlags;
+    filterResetCache();
 }
 
 LEGlyphID *GlyphIterator::insertGlyphs(le_int32 count, LEErrorCode& success)
@@ -381,53 +386,68 @@
     glyphPositionAdjustments->setCursiveGlyph(position, baselineIsLogicalEnd());
 }
 
-le_bool GlyphIterator::filterGlyph(le_uint32 index) const
-{
-    LEErrorCode success = LE_NO_ERROR;
-    LEGlyphID glyphID = glyphStorage[index];
-    le_int32 glyphClass = gcdNoGlyphClass;
-
-    if (LE_GET_GLYPH(glyphID) >= 0xFFFE) {
-        return TRUE;
+void GlyphIterator::filterResetCache(void) {
+  filterCacheValid = FALSE;
     }
 
+le_bool GlyphIterator::filterGlyph(le_uint32 index)
+{
+    LEGlyphID glyphID = glyphStorage[index];
+
+    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;
+        }
+    }
+          break;
+
+    case gcdComponentGlyph:
+          filterResult = ((lookupFlags & lfIgnoreBaseGlyphs) != 0);
+          break;
+
+    default:
+          filterResult = FALSE;
+          break;
+    }
+      }
+      filterCacheValid = TRUE;
         }
 
-        return FALSE;
-    }
-
-    case gcdComponentGlyph:
-        return (lookupFlags & lfIgnoreBaseGlyphs) != 0;
-
-    default:
-        return FALSE;
-    }
+    return filterCache.result;
 }
 
 le_bool GlyphIterator::hasFeatureTag(le_bool matchGroup) const
--- a/src/share/native/sun/font/layout/GlyphIterator.h	Wed Jul 03 17:54:38 2013 +0100
+++ b/src/share/native/sun/font/layout/GlyphIterator.h	Wed Jul 03 14:50:03 2013 -0700
@@ -98,7 +98,7 @@
     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 @@
     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
--- a/src/share/native/sun/font/layout/LETableReference.h	Wed Jul 03 17:54:38 2013 +0100
+++ b/src/share/native/sun/font/layout/LETableReference.h	Wed Jul 03 14:50:03 2013 -0700
@@ -377,7 +377,7 @@
    * @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();
@@ -385,31 +385,31 @@
   /**
    * 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;
   }
--- a/src/share/native/sun/font/layout/OpenTypeUtilities.cpp	Wed Jul 03 17:54:38 2013 +0100
+++ b/src/share/native/sun/font/layout/OpenTypeUtilities.cpp	Wed Jul 03 14:50:03 2013 -0700
@@ -79,6 +79,7 @@
 
 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 @@
   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 @@
   }
 
   {
-    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);
     }
   }