[HarfBuzz] an optimization for complex fonts

[Jonathan Kew]

Hey Behdad,

Turns out that for fonts such as Noto Devanagari and Gujarati, we're 
spending an inordinate amount of time under lookup.is_inplace 
(&inplace_c) in apply_string().

We can get a big win for these fonts if we cache the is_inplace result 
in the lookup-accelerator, instead of recursing down through the 
sub-lookups every time.

With the attached patch, I see the total run time for a test such as

   hb-shape NotoSansDevanagari-Regular.ttf --text-file hi.txt > /dev/null

drop from 22 seconds to 8 sec. And that includes the time to read the 
file, populate and serialize the buffer, etc., so once those constant 
factors are discounted, the actual shaping itself is over 3x faster. :)

NotoSansGujarati shows a similar gain; and Amiri with ar.txt runs in 45 
sec instead of 1 min+, for a speedup of over 25%.

(For fonts that don't have lots of complex contextual lookups, of 
course, there's no significant difference.)

See what you think. An alternative implementation might be to initialize 
the is_inplace flag during hb_ot_layout_lookup_accelerator_t::init() 
(then no need for the _initialized flag, hence taking an if() out of the 
hot path, and the hb_ot_layout_lookup_accelerator_t references could 
remain const), but it looked like this would involve rather more 
rearrangement of code, so I took the simplest approach for now.

JK
-------------- next part --------------
diff --git a/src/hb-ot-layout-private.hh b/src/hb-ot-layout-private.hh
index 139e33f..ae5bb99 100644
--- a/src/hb-ot-layout-private.hh
+++ b/src/hb-ot-layout-private.hh
@@ -86,7 +86,7 @@ namespace OT {
 HB_INTERNAL void
 hb_ot_layout_substitute_lookup (OT::hb_apply_context_t *c,
 				const OT::SubstLookup &lookup,
-				const hb_ot_layout_lookup_accelerator_t &accel);
+				hb_ot_layout_lookup_accelerator_t &accel);
 
 
 /* Should be called after all the substitute_lookup's are done */
@@ -124,6 +124,7 @@ struct hb_ot_layout_lookup_accelerator_t
   {
     digest.init ();
     lookup.add_coverage (&digest);
+    is_inplace_initialized = false;
   }
 
   template <typename TLookup>
@@ -132,6 +133,8 @@ struct hb_ot_layout_lookup_accelerator_t
   }
 
   hb_set_digest_t digest;
+  bool is_inplace;
+  bool is_inplace_initialized;
 };
 
 struct hb_ot_layout_t
diff --git a/src/hb-ot-layout.cc b/src/hb-ot-layout.cc
index bd8ef08..be17701 100644
--- a/src/hb-ot-layout.cc
+++ b/src/hb-ot-layout.cc
@@ -790,7 +790,7 @@ struct GSUBProxy
     accels (hb_ot_layout_from_face (face)->gsub_accels) {}
 
   const OT::GSUB &table;
-  const hb_ot_layout_lookup_accelerator_t *accels;
+  hb_ot_layout_lookup_accelerator_t *accels;
 };
 
 struct GPOSProxy
@@ -803,7 +803,7 @@ struct GPOSProxy
     accels (hb_ot_layout_from_face (face)->gpos_accels) {}
 
   const OT::GPOS &table;
-  const hb_ot_layout_lookup_accelerator_t *accels;
+  hb_ot_layout_lookup_accelerator_t *accels;
 };
 
 
@@ -820,11 +820,9 @@ template <typename Proxy>
 static inline bool
 apply_string (OT::hb_apply_context_t *c,
 	      const typename Proxy::Lookup &lookup,
-	      const hb_ot_layout_lookup_accelerator_t &accel)
+	      hb_ot_layout_lookup_accelerator_t &accel)
 {
   bool ret = false;
-  OT::hb_is_inplace_context_t inplace_c (c->face);
-  bool inplace = lookup.is_inplace (&inplace_c);
   hb_buffer_t *buffer = c->buffer;
 
   if (unlikely (!buffer->len || !c->lookup_mask))
@@ -850,6 +848,18 @@ apply_string (OT::hb_apply_context_t *c,
     }
     if (ret)
     {
+      bool inplace;
+      if (accel.is_inplace_initialized)
+      {
+	inplace = accel.is_inplace;
+      }
+      else
+      {
+	accel.is_inplace_initialized = true;
+	OT::hb_is_inplace_context_t inplace_c (c->face);
+	inplace = accel.is_inplace = lookup.is_inplace (&inplace_c);
+      }
+
       if (!inplace)
 	buffer->swap_buffers ();
       else
@@ -924,7 +934,7 @@ void hb_ot_map_t::position (const hb_ot_shape_plan_t *plan, hb_font_t *font, hb_
 HB_INTERNAL void
 hb_ot_layout_substitute_lookup (OT::hb_apply_context_t *c,
 				const OT::SubstLookup &lookup,
-				const hb_ot_layout_lookup_accelerator_t &accel)
+				hb_ot_layout_lookup_accelerator_t &accel)
 {
   apply_string<GSUBProxy> (c, lookup, accel);
 }