Improve font enumeration performance

[GoogleCodeExporter]

In testing for 19, when the font system goes through and throws away bitmap 
only fonts, this causes a lot of system churn which is unnecessary because it 
happens more frequently than we realise. It should be possible to cache this.

Original issue reported on code.google.com by classi...@floodgap.com on 15 Dec 2012 at 5:53

• Blocking: #171

[GoogleCodeExporter]

We could keep an array of ATSFontRefs that failed.

Original comment by classi...@floodgap.com on 15 Dec 2012 at 6:04

[GoogleCodeExporter]

This looks like an instance of 
https://bugzilla.mozilla.org/show_bug.cgi?id=705594

Original comment by classi...@floodgap.com on 15 Dec 2012 at 11:35

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

This appears to be an issue with fallback. Perhaps if we made HasFontTable 
faster.

Original comment by classi...@floodgap.com on 15 Dec 2012 at 11:59

[GoogleCodeExporter]

Wrote up a new HasFontTable that instead of doing an expensive ATSFontGetTable 
just to check for its existence, instead grabs ATSFontGetTableDirectory and 
parses it. This is much, much faster and speeds up font enumeration overall as 
well as rejections.

Although it could be made AltiVec, we're only scanning around 300-400 bytes 
(which get turned into uint32_ts anyway), so it doesn't seem worth the 
overhead. Rewriting ReadCMAP() to cache the directory and search results does 
seem worth it later on, although this is already a lot quicker.

Original comment by classi...@floodgap.com on 17 Dec 2012 at 4:46

• Changed state: Started

[GoogleCodeExporter]

Original comment by classi...@floodgap.com on 17 Dec 2012 at 4:49

• Now blocking: #73

[GoogleCodeExporter]

Original comment by classi...@floodgap.com on 17 Dec 2012 at 4:50

• Now blocking: #171

[GoogleCodeExporter]

bool
MacOSFontEntry::HasFontTable(uint32_t aTableTag)
{
    // ATSFontRef version
    // This is higher performance than the previous version.

    // XXX future: split out the scanning routine as a static function,
    // then allow functions that call this a lot to get and cache the
    // directory and scan it themselves. ReadCMAP() is an optimal example.

    ATSFontRef fontRef = GetATSFontRef();
    ByteCount sizer;

    if (fontRef == kInvalidFont) return false;

    if(::ATSFontGetTableDirectory(fontRef, 0, NULL, &sizer) == noErr) {
      if (!sizer) return false; // wtf?

      // If the header is abnormally short, try the old, slower way. (?)
      if (sizer <= 12) // Woof!
        return
        (::ATSFontGetTable(fontRef, aTableTag, 0, 0, 0, &sizer) == noErr);

      // Get the font table directory.
      ByteCount sizer2 = sizer;
      AutoFallibleTArray<uint8_t,65536> table;
      table.SetLength(sizer2);
#ifdef DEBUG
      uint32_t j = 12;
      fprintf(stderr, "Size of directory: %i\nTables: ", table.Length());
#endif
      if (::ATSFontGetTableDirectory(fontRef, sizer2,
                reinterpret_cast<void *>(table.Elements()), &sizer) == noErr) {
        // Parse it. In big endian format, each entry is 4 32-bit words
        // corresponding to the tag, checksum, offset and length, with a
        // 96 bit header (three 32-bit words). One day we could even write
        // an AltiVec version ...
        uint32_t i;
        uint32_t *wtable = (reinterpret_cast<uint32_t *>(table.Elements()));
        for (i=3; i<(sizer/4); i+=4) { // Skip header
#ifdef DEBUG
        char tag[5] = { table[j], table[j+1], table[j+2], table[j+3], '\0' };
        fprintf(stderr, "%s ", tag); // remember: big endian
        j+=16;
#endif
          // ASSUME THAT aTableTag is already big endian!!! XXX???
          if(wtable[i] == aTableTag) {
#ifdef DEBUG
                fprintf(stderr, "MATCH.\n");
#endif
                return true;
          }
        }
      }
    }

    // Hmmm. Either something is wrong, or there is no table. So no table.
#ifdef DEBUG
    fprintf(stderr, "\nNO MATCH\n");
#endif
    return false;
}

Original comment by classi...@floodgap.com on 17 Dec 2012 at 4:56

[GoogleCodeExporter]

I think I'll add some better bounds checking. sizer <= 12 || ((sizer-12) % 16)
And then instrument it to flag bogus fonts requiring full ::ATSFontGetTable, 
even if not in debug mode.

Original comment by classi...@floodgap.com on 17 Dec 2012 at 5:54

• Added labels: Milestone-NextStable

[GoogleCodeExporter]

This is not nearly fast enough. I think we should cache the table directory in 
the MacOSFontEntry. So I'm going to implement that.

Original comment by classi...@floodgap.com on 21 Dec 2012 at 3:56

[GoogleCodeExporter]

Spun scanner routine into a static routine, and now HasFontTable just stubs 
into a quick table dir search if the table dir had been previously fetched. 
Even better.

Original comment by classi...@floodgap.com on 21 Dec 2012 at 4:31

[GoogleCodeExporter]

/* in MacOSFontEntry (for 17: ATSFontEntry)
    AutoFallibleTArray<uint8_t,1024> mFontTableDir; // 10.4Fx
    ByteCount mFontTableDirSize; // 10.4Fx
*/

static bool FindTagInTableDir(FallibleTArray<uint8_t>& table, 
                uint32_t aTableTag, ByteCount sizer) {
  // Parse it. In big endian format, each entry is 4 32-bit words
  // corresponding to the tag, checksum, offset and length, with a
  // 96 bit header (three 32-bit words). One day we could even write
  // an AltiVec version ...
#ifdef DEBUG
  uint32_t j = 12;
#endif
  uint32_t i;
  uint32_t *wtable = (reinterpret_cast<uint32_t *>(table.Elements()));
  for (i=3; i<(sizer/4); i+=4) { // Skip header
#ifdef DEBUG
    char tag[5] = { table[j], table[j+1], table[j+2], table[j+3], '\0' };
    fprintf(stderr, "%s ", tag); // remember: big endian
    j+=16;
#endif
    // ASSUME THAT aTableTag is already big endian!!! XXX???
    if(wtable[i] == aTableTag) {
#ifdef DEBUG
      fprintf(stderr, "MATCH\n");
#endif
      return true;
    }
  }
  // Hmmm. Either something is wrong, or there is no table. So no table.
#ifdef DEBUG
  fprintf(stderr, "NO MATCH\n");
#endif
  return false;
}

bool
MacOSFontEntry::HasFontTable(uint32_t aTableTag)
{
    // ATSFontRef version
    // This is higher performance than the previous version.

    ATSFontRef fontRef = GetATSFontRef();
    if (fontRef == kInvalidFont) return false;

    // Use cached directory to avoid repeatedly fetching the same data.
    if (mFontTableDirSize > 0)
        return FindTagInTableDir(mFontTableDir, aTableTag, mFontTableDirSize);

    ByteCount sizer;

    if(::ATSFontGetTableDirectory(fontRef, 0, NULL, &sizer) == noErr) {

      // If the header is abnormal, try the old, slower way in case this
      // is a gap in our algorithm.
      if (sizer <= 12 || ((sizer-12) % 16) || sizer >= 1024) {
        fprintf(stderr, "Warning: TenFourFox found "
                "abnormal font table dir in %s (%i).\n",
                 NS_ConvertUTF16toUTF8(mName).get(), sizer);
        return
        (::ATSFontGetTable(fontRef, aTableTag, 0, 0, 0, &sizer) == noErr);
      }

      // Get and cache the font table directory.
      mFontTableDirSize = sizer;
      mFontTableDir.SetLength(mFontTableDirSize);
#ifdef DEBUG
      fprintf(stderr, "Size of %s font table directory: %i\nTables: ",
                NS_ConvertUTF16toUTF8(mName).get(), table.Length());
#endif
      if (::ATSFontGetTableDirectory(fontRef, mFontTableDirSize,
        reinterpret_cast<void *>(mFontTableDir.Elements()), &sizer) == noErr)
        return FindTagInTableDir(mFontTableDir, aTableTag, mFontTableDirSize);
   }
   // Something is really wrong. Trash the cache, give up.
   mFontTableDirSize = 0;
   fprintf(stderr, "Warning: TenFourFox cannot process font %s.\n",
                 NS_ConvertUTF16toUTF8(mName).get());
   return false; 
}

Original comment by classi...@floodgap.com on 21 Dec 2012 at 4:55

[GoogleCodeExporter]

Landed in 17.0.2 also

Original comment by classi...@floodgap.com on 5 Jan 2013 at 1:27

[GoogleCodeExporter]

Issue 201 has been merged into this issue.

Original comment by classi...@floodgap.com on 5 Jan 2013 at 11:46

[GoogleCodeExporter]

This makes 17 crash. A call-stack that makes sense:

Warning: TenFourFox rejecting bitmap-only font LucidaGrande.
Warning: TenFourFox rejecting bitmap-only font LucidaGrande-Bold.

Program received signal EXC_BAD_ACCESS, Could not access memory.
Reason: KERN_INVALID_ADDRESS at address: 0x165ff008
0x0343fb70 in ATSFontEntry::HasFontTable (this=0x19daa00, aTableTag=1836020344) 
at /Volumes/BruceDeuce/src/esr17c/gfx/thebes/gfxMacPlatformFontList.mm:512
512         if(wtable[i] == aTableTag) {
(gdb) bt
#0  0x0343fb70 in ATSFontEntry::HasFontTable (this=0x19daa00, 
aTableTag=1836020344) at 
/Volumes/BruceDeuce/src/esr17c/gfx/thebes/gfxMacPlatformFontList.mm:512
#1  0x0343f304 in MacOSFontEntry::ReadCMAP (this=0x19daa00) at 
/Volumes/BruceDeuce/src/esr17c/gfx/thebes/gfxMacPlatformFontList.mm:308
#2  0x03409dcc in gfxFontEntry::TestCharacterMap (this=0x19daa00, aCh=67) at 
/Volumes/BruceDeuce/src/esr17c/gfx/thebes/gfxFont.cpp:95

Original comment by classi...@floodgap.com on 5 Jan 2013 at 11:47

[GoogleCodeExporter]

OMFG I am a total moron. I'm not initializing mFontTableDirSize.

Original comment by classi...@floodgap.com on 6 Jan 2013 at 2:35

[GoogleCodeExporter]

And now it works.

Original comment by classi...@floodgap.com on 6 Jan 2013 at 2:47

[GoogleCodeExporter]

Original comment by classi...@floodgap.com on 6 Jan 2013 at 9:56

• Changed title: Improve font enumeration performance
• Changed state: Verified

[GoogleCodeExporter]

Trying to merge this into AuroraFox I noticed that ATSFontGetTable is indeed 
called twice in GetFontTable as well (and in TenFourFox there is also 
gfxMacFont::GetFontTable calling this, if I managed to interpret the patchfiles 
correctly). The first call is done in order to get the size of memory that 
needs to be allocated to be able to actually read the table data with the 
second call.
Now ATSFontGetTableDirectory gets us the table directory entry for a tag, which 
already contains the table length as its last element. The code pasted below 
shows how I use FindTagInTableDir to get the table length which is then used to 
avoid calling ATSFontGetTable twice.
(I'm not sure about the autorelease pools, but I think they aren't necessary 
for Carbon code.)

static bool FindTagInTableDir(FallibleTArray<uint8_t>& table, 
                              uint32_t aTableTag, ByteCount size,
                              ByteCount& tableLength)
{
    // Parse it. In big endian format, each entry is 4 32-bit words
    // corresponding to the tag, checksum, offset and length, with a
    // 96 bit header (three 32-bit words). One day we could even write
    // an AltiVec version ...
#ifdef DEBUG
    uint32_t j = 12;
#endif
    uint32_t i;
    uint32_t* wtable = (reinterpret_cast<uint32_t *>(table.Elements()));
    for (i=3; i<(size/4); i+=4) { // Skip header
#ifdef DEBUG
        char tag[5] = { table[j], table[j+1], table[j+2], table[j+3], '\0' };
        fprintf(stderr, "%s ", tag); // remember: big endian
        j+=16;
#endif
        // ASSUME THAT aTableTag is already big endian!!! XXX???
        if(wtable[i] == aTableTag) {
#ifdef DEBUG
            fprintf(stderr, "MATCH\n");
#endif
            tableLength = wtable[i+3];
            return true;
        }
    }
    // Hmmm. Either something is wrong, or there is no table. So no table.
#ifdef DEBUG
    fprintf(stderr, "NO MATCH\n");
#endif
    return false;
}

nsresult
ATSFontEntry::GetFontTable(uint32_t aTableTag, FallibleTArray<uint8_t>& aBuffer)
{
    nsAutoreleasePool localPool;

    ByteCount tableLength;
    if (!HasFontTable(aTableTag, tableLength)) {
        return NS_ERROR_FAILURE;
    }

    if (!aBuffer.SetLength(tableLength)) {
        return NS_ERROR_OUT_OF_MEMORY;
    }
    uint8_t *dataPtr = aBuffer.Elements();

    OSStatus status = ::ATSFontGetTable(GetATSFontRef(), aTableTag, 0, tableLength, dataPtr, &tableLength);
    NS_ENSURE_TRUE(status == noErr, NS_ERROR_FAILURE);

    return NS_OK;
}

inline bool
ATSFontEntry::HasFontTable(uint32_t aTableTag, ByteCount& tableLength)
{
    ATSFontRef fontRef = GetATSFontRef();
    ByteCount size;
    if (fontRef == kInvalidFont)
        return false;

    // Use cached directory to avoid repeatedly fetching the same data.
    if (mFontTableDirSize > 0)
        return FindTagInTableDir(mFontTableDir, aTableTag, mFontTableDirSize, tableLength);

    if(::ATSFontGetTableDirectory(fontRef, 0, NULL, &size) == noErr) {
        // If the header is abnormal, try the old, slower way in case this
        // is a gap in our algorithm.
        if (size <= 12 || ((size-12) % 16) || size >= 1024) {
            fprintf(stderr, "Warning: TenFourFox found "
                            "abnormal font table dir in %s (%i).\n",
                    NS_ConvertUTF16toUTF8(mName).get(), size);
            return
                (::ATSFontGetTable(fontRef, aTableTag, 0, 0, 0, &size) == noErr);
        }

        // Get and cache the font table directory.
        mFontTableDirSize = size;
        mFontTableDir.SetLength(mFontTableDirSize);
#ifdef DEBUG
        fprintf(stderr, "Size of %s font table directory: %i\nTables: ",
                NS_ConvertUTF16toUTF8(mName).get(), mFontTableDir.Length());
#endif
        if (::ATSFontGetTableDirectory(fontRef, mFontTableDirSize,
            reinterpret_cast<void *>(mFontTableDir.Elements()), &size) == noErr)
            return
                FindTagInTableDir(mFontTableDir, aTableTag, mFontTableDirSize, tableLength);
     }
     // Something is really wrong. Trash the cache, give up.
     mFontTableDirSize = 0;
     fprintf(stderr, "Warning: TenFourFox cannot process font %s.\n",
             NS_ConvertUTF16toUTF8(mName).get());
     return false;
}

bool
ATSFontEntry::HasFontTable(uint32_t aTableTag)
{
    nsAutoreleasePool localPool;
    ByteCount tableLength;
    return HasFontTable(aTableTag, tableLength);
}

Original comment by Tobias.N...@gmail.com on 14 Jan 2013 at 9:12

[GoogleCodeExporter]

How much of a difference does this make? It should be a separate issue though.

Original comment by classi...@floodgap.com on 14 Jan 2013 at 10:21

[GoogleCodeExporter]

I don't know how to measure but before finally calling HasFontTable it does 
always call one or several times GetFontTable for different tags. So that way 
upon the first call to HasFontTable the directory has already been cached 
during a call to GetFontTable for the same tag.

Original comment by Tobias.N...@gmail.com on 14 Jan 2013 at 11:48

[GoogleCodeExporter]

Changes to the MacOSFontEntry object in Fx24 make adding your approach trivial, 
so I'm doing that for 24 in a modified form.

Original comment by classi...@floodgap.com on 6 Jul 2013 at 7:53

[GoogleCodeExporter]

Original comment by classi...@floodgap.com on 29 Oct 2013 at 4:48