Caches grob properties before evaluation. (issue 6573066)

[mtsolo]

Reviewers: ,

Message:
This slows down LilyPond - I haven't done comprehensive tests of how
much.  I'm pretty sure it works (the regtest works as expected).
Irrespective of how multiple passes are done, this seems like a
necessary first step.

Note that this patch would not have a time impact if it used properties
stashed in immutable_properties_alist_. If all side effects were
eliminated from LilyPond (which would be awesome), then this could be
done.  However, this would be very difficult - for example, tweaks are
given to grobs after their immutable_properties_alist_ is fixed (see the
tweak engraver).

Description:
Caches grob properties before evaluation.

We cannot simply use the original value stored in the
immutable_properties_alist_ because simple closures may be created
at the engraver stage via chain_offset_callback and related functions.
Instead, we grab the property data at the moment of evaluation and cache
it.

Please review this at https://codereview.appspot.com/6573066/

Affected files:
  A input/regression/restore-property-to-original-value.ly
  M lily/grob-property.cc
  M lily/grob-scheme.cc
  M lily/grob-smob.cc
  M lily/grob.cc
  M lily/include/grob.hh
  M lily/include/lily-guile-macros.hh


Index: input/regression/restore-property-to-original-value.ly
diff --git a/input/regression/restore-property-to-original-value.ly  
b/input/regression/restore-property-to-original-value.ly
new file mode 100644
index  
0000000000000000000000000000000000000000..1a7c1df1e1e7bd190f844ad5b6e4d7068147e775
--- /dev/null
+++ b/input/regression/restore-property-to-original-value.ly
@@ -0,0 +1,24 @@
+\version "2.17.4"
+
+\header {
+  texidoc = "Properties can be restored to their original values.
+Below, the @code{NoteHead} stencil should print but its extent
+should not be taken into account in horizontal spacing, as horizontal
+spacing happens between calls to @code{before-line-breaking} and
address@hidden
+"
+}
+
+\relative c'' {
+  \override NoteHead #'before-line-breaking =
+    #(lambda (grob)
+       ; trigger cache
+       (ly:grob-property grob 'stencil)
+       ; change the property
+       (ly:grob-set-property! grob 'stencil #f))
+  \override NoteHead #'after-line-breaking =
+    #(lambda (grob)
+       ; restore the property
+       (ly:grob-restore-property-to-original-value! grob 'stencil))
+  a
+}
Index: lily/grob-property.cc
diff --git a/lily/grob-property.cc b/lily/grob-property.cc
index  
3afe182c0ef07793ecf1fd0d09a5ea147c9bd4e1..826b1e3f11d19d4aaaca7e13de65f20326e1b9d7  
100644
--- a/lily/grob-property.cc
+++ b/lily/grob-property.cc
@@ -164,6 +164,15 @@ SCM
 Grob::internal_get_property (SCM sym) const
 {
   SCM val = get_property_data (sym);
+  // if this is our first lookup, set the cache
+  SCM cached_handle = scm_sloppy_assq (sym,
+                                       cached_mutable_property_alist_);
+  if (cached_handle == SCM_BOOL_F)
+    {
+      Grob *me = ((Grob *)this);
+      me->internal_set_value_on_alist (&me->cached_mutable_property_alist_,
+                                       sym, val);
+    }

 #ifndef NDEBUG
   if (val == ly_symbol2scm ("calculation-in-progress"))
@@ -211,6 +220,21 @@ Grob::internal_get_maybe_pure_property (SCM sym, bool  
pure, int start, int end)
   return pure ? internal_get_pure_property (sym, start, end) :  
internal_get_property (sym);
 }

+void
+Grob::internal_restore_property_to_original_value (SCM sym)
+{
+  SCM handle = scm_sloppy_assq (sym, cached_mutable_property_alist_);
+  // We could issue warning if handle == SCM_BOOL_F, but it is possible
+  // that one wants to restore an original property before the property
+  // has been evaluated.  By definition, this means that the original
+  // property is still there.
+
+  if (handle != SCM_BOOL_F)
+    internal_set_value_on_alist (&mutable_property_alist_,
+                                 sym, scm_cdr (handle));
+
+}
+
 SCM
 Grob::try_callback_on_alist (SCM *alist, SCM sym, SCM proc)
 {
Index: lily/grob-scheme.cc
diff --git a/lily/grob-scheme.cc b/lily/grob-scheme.cc
index  
81ff7864ff36b4b4ff972bb0fc3051d8700f60e5..e144be6f6a0884acc07f7f7408e59834731314e5  
100644
--- a/lily/grob-scheme.cc
+++ b/lily/grob-scheme.cc
@@ -179,6 +179,20 @@ LY_DEFINE (ly_grob_set_object_x, "ly:grob-set-object!",
   return SCM_UNSPECIFIED;
 }

+LY_DEFINE  
(ly_grob_restore_property_to_original_value_x, "ly:grob-restore-property-to-original-value!",
+           2, 0, 0, (SCM grob, SCM sym),
+           "Restore the property for property @var{sym} of @var{grob}."
+           " to its original value.")
+{
+  Grob *sc = unsmob_grob (grob);
+
+  LY_ASSERT_SMOB (Grob, grob, 1);
+  LY_ASSERT_TYPE (ly_is_symbol, sym, 2);
+
+  sc->restore_property_to_original_value (sym);
+  return SCM_UNSPECIFIED;
+}
+
 /* TODO: make difference between scaled and unscalead variable in
    calling (i.e different funcs.) */
 LY_DEFINE (ly_grob_layout, "ly:grob-layout",
Index: lily/grob-smob.cc
diff --git a/lily/grob-smob.cc b/lily/grob-smob.cc
index  
853546a8868d547d5836e76514a40908abf43bbe..c4c9fd6a37040fb01340a5cf12d00a3b9f0c1b42  
100644
--- a/lily/grob-smob.cc
+++ b/lily/grob-smob.cc
@@ -48,6 +48,7 @@ Grob::mark_smob (SCM ses)

   s->derived_mark ();
   scm_gc_mark (s->object_alist_);
+  scm_gc_mark (s->cached_mutable_property_alist_);
   scm_gc_mark (s->interfaces_);

   return s->mutable_property_alist_;
Index: lily/grob.cc
diff --git a/lily/grob.cc b/lily/grob.cc
index  
031af32636878b3a163bb0ea483bca2e0d0fcdc5..2ba3904b46473d8c15204ecf14e34b2e158fb67a  
100644
--- a/lily/grob.cc
+++ b/lily/grob.cc
@@ -57,6 +57,7 @@ Grob::Grob (SCM basicprops)
   immutable_property_alist_ = basicprops;
   mutable_property_alist_ = SCM_EOL;
   object_alist_ = SCM_EOL;
+  cached_mutable_property_alist_ = SCM_EOL;

   /* We do smobify_self () as the first step.  Since the object lives
      on the heap, none of its SCM variables are protected from
@@ -94,6 +95,7 @@ Grob::Grob (Grob const &s)

   immutable_property_alist_ = s.immutable_property_alist_;
   mutable_property_alist_ = SCM_EOL;
+  cached_mutable_property_alist_ = SCM_EOL;
   interfaces_ = s.interfaces_;
   object_alist_ = SCM_EOL;

@@ -102,6 +104,7 @@ Grob::Grob (Grob const &s)
   smobify_self ();

   mutable_property_alist_ = ly_deep_copy (s.mutable_property_alist_);
+  cached_mutable_property_alist_ = ly_deep_copy  
(s.mutable_property_alist_);

 }

@@ -270,6 +273,7 @@ Grob::suicide ()

   mutable_property_alist_ = SCM_EOL;
   object_alist_ = SCM_EOL;
+  cached_mutable_property_alist_ = SCM_EOL;
   immutable_property_alist_ = SCM_EOL;
   interfaces_ = SCM_EOL;
 }
Index: lily/include/grob.hh
diff --git a/lily/include/grob.hh b/lily/include/grob.hh
index  
09cd566ad335b39708990d05ab74894ff2099af1..b76bc6046f88e86558bf4930cb84ac86c4e9fe98  
100644
--- a/lily/include/grob.hh
+++ b/lily/include/grob.hh
@@ -44,6 +44,7 @@ protected:
   /* SCM data */
   SCM immutable_property_alist_;
   SCM mutable_property_alist_;
+  SCM cached_mutable_property_alist_;
   SCM object_alist_;

   /*
@@ -107,6 +108,7 @@ public:
   void internal_del_property (SCM symbol);
   void instrumented_set_property (SCM, SCM, char const *, int, char const  
*);
   void internal_set_property (SCM sym, SCM val);
+  void internal_restore_property_to_original_value (SCM sym);

   /* messages */
   void warning (string) const;
Index: lily/include/lily-guile-macros.hh
diff --git a/lily/include/lily-guile-macros.hh  
b/lily/include/lily-guile-macros.hh
index  
c0df65849824051b13d50f5dcd3d9dc748abcfef..cce5677815e8e95b1acef59d0194a64e1c82aa69  
100644
--- a/lily/include/lily-guile-macros.hh
+++ b/lily/include/lily-guile-macros.hh
@@ -212,6 +212,8 @@ void ly_check_name (string cxx, string fname);
 #define get_object(x) internal_get_object (ly_symbol2scm (x))
 #define set_object(x, y) internal_set_object (ly_symbol2scm (x), y)
 #define del_property(x) internal_del_property (ly_symbol2scm (x))
+#define restore_property_to_original_value(x) \
+  internal_restore_property_to_original_value (ly_symbol2scm (x))

 #ifndef NDEBUG
 /*