Bug 1575713 - Use Span<> rather than copying to return stuff from gfxFontFeatureValueSet. r=boris
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 22 Aug 2019 00:24:47 +0000
changeset 489336 c44b510774123df58588ef9b41c4ec0bd204b473
parent 489335 8ca157711117d3af9716bc7f3f19125b8957def5
child 489337 d779671a20bf506ee323b8f2f6210b35ab2da0c8
push id36469
push useraiakab@mozilla.com
push dateThu, 22 Aug 2019 09:50:44 +0000
treeherdermozilla-central@152bfc05a509 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersboris
bugs1575713
milestone70.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
Bug 1575713 - Use Span<> rather than copying to return stuff from gfxFontFeatureValueSet. r=boris It's a really minor perf improvement, but also a cleanup IMO. Also be a bit more const-correct while at it. Differential Revision: https://phabricator.services.mozilla.com/D42985
gfx/thebes/gfxFont.cpp
gfx/thebes/gfxFontFeatures.cpp
gfx/thebes/gfxFontFeatures.h
--- a/gfx/thebes/gfxFont.cpp
+++ b/gfx/thebes/gfxFont.cpp
@@ -326,61 +326,56 @@ void gfxFontCache::AddSizeOfIncludingThi
                                           FontCacheSizes* aSizes) const {
   aSizes->mFontInstances += aMallocSizeOf(this);
   AddSizeOfExcludingThis(aMallocSizeOf, aSizes);
 }
 
 #define MAX_SSXX_VALUE 99
 #define MAX_CVXX_VALUE 99
 
-static void LookupAlternateValues(gfxFontFeatureValueSet& aFeatureLookup,
+static void LookupAlternateValues(const gfxFontFeatureValueSet& aFeatureLookup,
                                   const nsACString& aFamily,
                                   const StyleVariantAlternates& aAlternates,
                                   nsTArray<gfxFontFeature>& aFontFeatures) {
   using Tag = StyleVariantAlternates::Tag;
 
   // historical-forms gets handled in nsFont::AddFontFeaturesToStyle.
   if (aAlternates.IsHistoricalForms()) {
     return;
   }
 
   gfxFontFeature feature;
   if (aAlternates.IsCharacterVariant()) {
     for (auto& ident : aAlternates.AsCharacterVariant().AsSpan()) {
-      AutoTArray<uint32_t, 4> values;
-      // FIXME(emilio): Use atoms directly.
-      aFeatureLookup.GetFontFeatureValuesFor(
+      Span<const uint32_t> values = aFeatureLookup.GetFontFeatureValuesFor(
           aFamily, NS_FONT_VARIANT_ALTERNATES_CHARACTER_VARIANT,
-          ident.AsAtom(), values);
+          ident.AsAtom());
       // nothing defined, skip
       if (values.IsEmpty()) {
         continue;
       }
       NS_ASSERTION(values.Length() <= 2,
                    "too many values allowed for character-variant");
       // character-variant(12 3) ==> 'cv12' = 3
       uint32_t nn = values[0];
       // ignore values greater than 99
       if (nn == 0 || nn > MAX_CVXX_VALUE) {
         continue;
       }
-      feature.mValue = values.SafeElementAt(1, 1);
+      feature.mValue = values.Length() > 1 ? values[1] : 1;
       feature.mTag = HB_TAG('c', 'v', ('0' + nn / 10), ('0' + nn % 10));
       aFontFeatures.AppendElement(feature);
     }
     return;
   }
 
   if (aAlternates.IsStyleset()) {
     for (auto& ident : aAlternates.AsStyleset().AsSpan()) {
-      AutoTArray<uint32_t, 4> values;
-      // FIXME(emilio): Use atoms directly.
-      aFeatureLookup.GetFontFeatureValuesFor(
-          aFamily, NS_FONT_VARIANT_ALTERNATES_STYLESET,
-          ident.AsAtom(), values);
+      Span<const uint32_t> values = aFeatureLookup.GetFontFeatureValuesFor(
+          aFamily, NS_FONT_VARIANT_ALTERNATES_STYLESET, ident.AsAtom());
 
       // styleset(1 2 7) ==> 'ss01' = 1, 'ss02' = 1, 'ss07' = 1
       feature.mValue = 1;
       for (uint32_t nn : values) {
         if (nn == 0 || nn > MAX_SSXX_VALUE) {
           continue;
         }
         feature.mTag = HB_TAG('s', 's', ('0' + nn / 10), ('0' + nn % 10));
@@ -409,18 +404,18 @@ static void LookupAlternateValues(gfxFon
       constant = NS_FONT_VARIANT_ALTERNATES_ANNOTATION;
       name = aAlternates.AsAnnotation().AsAtom();
       break;
     default:
       MOZ_ASSERT_UNREACHABLE("Unknown font-variant-alternates value!");
       return;
   }
 
-  AutoTArray<uint32_t, 4> values;
-  aFeatureLookup.GetFontFeatureValuesFor(aFamily, constant, name, values);
+  Span<const uint32_t> values =
+      aFeatureLookup.GetFontFeatureValuesFor(aFamily, constant, name);
   if (values.IsEmpty()) {
     return;
   }
   MOZ_ASSERT(values.Length() == 1,
              "too many values for font-specific font-variant-alternates");
 
   feature.mValue = values[0];
   switch (aAlternates.tag) {
--- a/gfx/thebes/gfxFontFeatures.cpp
+++ b/gfx/thebes/gfxFontFeatures.cpp
@@ -7,33 +7,28 @@
 #include "nsAtom.h"
 #include "nsUnicharUtils.h"
 #include "nsHashKeys.h"
 
 using namespace mozilla;
 
 gfxFontFeatureValueSet::gfxFontFeatureValueSet() : mFontFeatureValues(8) {}
 
-bool gfxFontFeatureValueSet::GetFontFeatureValuesFor(
-    const nsACString& aFamily, uint32_t aVariantProperty,
-    nsAtom* aName, nsTArray<uint32_t>& aValues) {
+Span<const uint32_t> gfxFontFeatureValueSet::GetFontFeatureValuesFor(
+    const nsACString& aFamily, uint32_t aVariantProperty, nsAtom* aName) const {
   nsAutoCString family(aFamily);
   ToLowerCase(family);
   FeatureValueHashKey key(family, aVariantProperty, aName);
-
-  aValues.Clear();
   FeatureValueHashEntry* entry = mFontFeatureValues.GetEntry(key);
-  if (entry) {
-    NS_ASSERTION(entry->mValues.Length() > 0,
-                 "null array of font feature values");
-    aValues.AppendElements(entry->mValues);
-    return true;
+  if (!entry) {
+    return {};
   }
-
-  return false;
+  NS_ASSERTION(entry->mValues.Length() > 0,
+               "null array of font feature values");
+  return MakeSpan(entry->mValues);
 }
 
 nsTArray<uint32_t>* gfxFontFeatureValueSet::AppendFeatureValueHashEntry(
     const nsACString& aFamily, nsAtom* aName, uint32_t aAlternate) {
   FeatureValueHashKey key(aFamily, aAlternate, aName);
   FeatureValueHashEntry* entry = mFontFeatureValues.PutEntry(key);
   entry->mKey = key;
   return &entry->mValues;
--- a/gfx/thebes/gfxFontFeatures.h
+++ b/gfx/thebes/gfxFontFeatures.h
@@ -42,21 +42,19 @@ class gfxFontFeatureValueSet final {
     nsTArray<uint32_t> featureSelectors;
   };
 
   struct FeatureValues {
     uint32_t alternate;
     nsTArray<ValueList> valuelist;
   };
 
-  // returns true if found, false otherwise
-  bool GetFontFeatureValuesFor(const nsACString& aFamily,
-                               uint32_t aVariantProperty,
-                               nsAtom* aName,
-                               nsTArray<uint32_t>& aValues);
+  mozilla::Span<const uint32_t> GetFontFeatureValuesFor(
+      const nsACString& aFamily, uint32_t aVariantProperty,
+      nsAtom* aName) const;
 
   // Appends a new hash entry with given key values and returns a pointer to
   // mValues array to fill. This should be filled first.
   nsTArray<uint32_t>* AppendFeatureValueHashEntry(const nsACString& aFamily,
                                                   nsAtom* aName,
                                                   uint32_t aAlternate);
 
  private: