Menu
▾
▴
#3199 Patch: Uses only unpure-pure containers to articulate unpure-pure relationships.
Originally created by: *anonymous
Originally created by: mts...@gmail.com
Originally owned by: mts...@gmail.com
Uses only unpure-pure containers to articulate unpure-pure relationships.
Previously, LilyPond used several different lists in define-grobs.scm
to define relationships between unpure and pure functions. This patch
eliminates these lists, using unpure-pure containers to articulate
these relationships.
The modifications required to implement this change are described below:
-) axis-group-interface.cc
A Scheme function is no longer needed to determine pure relevant grobs.
All grobs can have their Y-extents meaningfully pure-evaluated now. The
worst-case scenario is that they evaluate to false. Dead grobs, on the
other hand, are never pure relevant. The calls to is_live are the only
holdovers from the old pure-relevant? scheme function.
-) grob-closure.cc
We allow unpure-pure containers in simple closures.
-) grob-property.cc
call_pure_function no longer looks up a Scheme module. Because there are
no hard-coded lists in Scheme any more, the function is smaller and
writing it in C++ gets slight efficiency gains.
-) grob.cc
pure_stencil_height used to be a Scheme function in define-grobs.scm.
Because it is much simpler (it no longer makes references to lists defined
in Scheme), it can be implemented in C++.
-) pure-from-neighbor-engraver.cc
Similar to axis-group-interface.cc, the pure-relevant distinction is
no longer important.
-) side-position-interface.cc
In order to avoid issues with alterBroken, we only check pure properties
before line breaking.
-) simple-closure.cc
Simple closures were incorrectly evaluated when containing unpure-pure
containers. This rectifies that.
-) stencil-integral.cc
Several pure equivalent functions needed to be written for skylines.
-) define-grobs.scm
Multiple overrides must be changed to unpure-pure containers. Previous
hard-coded lists are all deleted and several functions moved to C++ (see
above).
-) output-lib.scm
Several common unpure-pure containers used in define-grobs.scm are
defined here. Several functions from define-grobs.scm pertaining to
grob offsets are moved to this file.
Originally posted by: mts...@gmail.com
(No comment was entered for this change.)
Owner: mts...@gmail.com
Originally posted by: dak@gnu.org
Patchy the autobot says: passes tests. Documentation not tested, but it is only two functions with documentation strings and about 4 lines of code comments anyway. Lots of smaller profiling changes mostly to the worse. outside-staff-priority accesses increased by 30%. Several smaller typesetting changes like in tablature-chord-repetition, and tablature-dot-placement, tablature-full-notation. Is that the tab clef causing it? At any rate, apparently tablature-related. Regtest page-overflow-compression no longer triggers the condition that is supposed to demonstrate being able to handle.
Labels: -Patch-new Patch-review
Originally posted by: mts...@gmail.com
Setting to needs work - there is a big discrepancy in min-distances between paper columns for certain regtests. There are also different pure heights being reported. Oddly, this does not show up in the visual result - I caught it when spewing some data to the command line. It is the sort of thing that I want to understand before getting review.
Labels: -Patch-review Patch-needs_work
Originally posted by: mts...@gmail.com
Fixes side-position-interface::y-aligned-side unpure-pure-container
http://codereview.appspot.com/7377046
Labels: -Patch-needs_work Patch-new
Originally posted by: pkx1...@gmail.com
Patchy the autobot says: passes make, make test and a full make doc. Reg test diffs here - https://www.yousendit.com/download/UW14K2VwMGs4Q1FpR01UQw
Labels: -Patch-new Patch-review
Originally posted by: mts...@gmail.com
Fixes error in last patch set
http://codereview.appspot.com/7377046
Labels: -Patch-review Patch-new
Originally posted by: pkx1...@gmail.com
Patchy the autobot says: passes make, make test and a full make doc. Reg test diffs here - https://www.yousendit.com/download/UW14Z28rcTJnYU1sYzhUQw
Labels: -Patch-new Patch-review
Originally posted by: pkx1...@gmail.com
Patch on countdown for 27th Feb 2013 - 19:00 GMT
Labels: -Patch-review Patch-countdown
Originally posted by: mts...@gmail.com
Incorporates Keith's suggestion for repeat ties
http://codereview.appspot.com/7377046
Labels: -Patch-countdown Patch-new
Originally posted by: pkx1...@gmail.com
Patchy the autobot says: passes make, make test and a full make doc. Reg test diffs here https://www.yousendit.com/download/UW13T0NaMGtqY3BESjlVag
Labels: -Patch-new Patch-review
Originally posted by: mts...@gmail.com
Fixes Stencil check
http://codereview.appspot.com/7377046
Labels: -Patch-review Patch-new
Originally posted by: pkx1...@gmail.com
Patchy the autobot says: passes make, make test and a full make doc. Reg test diffs here https://www.yousendit.com/download/UW13NHArZDVuSlFpR01UQw
Labels: -Patch-new Patch-review
Originally posted by: pkx1...@gmail.com
On Patch Countdown for 5th March - 19:00 GMT
Labels: -Patch-review Patch-countdown
Originally posted by: mts...@gmail.com
Better pure height for span bars
http://codereview.appspot.com/7377046
Labels: -Patch-countdown Patch-new
Originally posted by: dak@gnu.org
Patchy the autobot says: passes tests. Hefty increases in memory usage in markup-commands, skyline-vertical-placement, page-spacing. Significant increases (> 10%) in property lookups like outside-staff-priority, cross-staff, Y-offset. tablature-full-notation is displayed as differing, but visual inspection does not really show anything obvious.
Labels: -Patch-new Patch-review
Originally posted by: pkx1...@gmail.com
Patchy the autobot says: passes make, make test and a full make doc. Reg test diffs here https://www.yousendit.com/download/UW14anZtcWZVVGtsYzlVag
Originally posted by: mts...@gmail.com
I'm not quite sure what to make of the markup-command increase - do you mean that markup stencil functions are being called more?
All the other increases are expected because:
a) more grobs now have pure height functions.
b) we are calling get_maybe_pure_property more in side-position-interface.cc. This can be mitigated by caching all pure properties (not just height) and proposed in another patchset if it is particularly detrimental (I don't have a sense of how much this 10% increase slows LilyPond down).
Originally posted by: dak@gnu.org
The cell usage is not an indication of performance (at least not directly) but rather of maximum memory usage. I've not taken a look at the tests themselves, but it would appear that more stuff accrues that is referenceable up to the end of the typesetting phase (possibly even beyond that but the tests don't show that).
Originally posted by: mts...@gmail.com
Ah, OK, well yes this makes sense as there are now lots of unpure-pure-containers, all of which are smobs that take up memory.
Originally posted by: dak@gnu.org
You have not made use of issue 3200 (which has been committed already) yet. An unpure-pure-container takes just two cells. Creating a closure like grob::wrap-in-unpure-pure-container does is more expensive, though it would seem that it is a one-time cost per grob type rather than per grob instance.
At any rate, the main incentive for doing issue 3200 was deobfuscation, so you should replace uses of grob::wrap-in-unpure-pure container with ly:make-unpure-pure-container and remove the definition.
Related
Issues:
#3200Originally posted by: mts...@gmail.com
When I do that, LilyPond hits an infinite loop.
Originally posted by: mts...@gmail.com
More specifically, it takes just replacing the first instance of grob::wrap-in-unpure-pure container in define-grobs.scm (for Accidental) and it hits this loop.
Originally posted by: dak@gnu.org
Did you remove the definition after/before the copy&replace? If you have
(define (ly:make-unpure-pure-container func)
(ly:make-unpure-pure-container ...))
left in, you are not going to be happy with the results.
That would be an obvious contender. Anything else causing an infinite loop points to a bug (possibly in my code) that should not be left in LilyPond.
Originally posted by: dak@gnu.org
Ok, is there anything that is going to become confused if unpure_pure_container_pure_part is going to return a fresh callback for every call not eq? to the last one? Is this going to get stuffed somewhere were it will not protected from garbage protection? Can you run this in a debugger, hit C-c and check where it is caught? That would be helpful.
Originally posted by: mts...@gmail.com
Nope, just replaced that one line.
#0 0x0012d422 in __kernel_vsyscall ()
#1 0x00618af9 in __lll_lock_wait () from /lib/tls/i686/cmov/libpthread.so.0
#2 0x0061413b in _L_lock_748 () from /lib/tls/i686/cmov/libpthread.so.0
#3 0x00613f61 in pthread_mutex_lock () from /lib/tls/i686/cmov/libpthread.so.0
#4 0x001d107c in scm_pthread_mutex_lock () from /usr/lib/libguile.so.17
#5 0x0017b77b in scm_gc_for_newcell () from /usr/lib/libguile.so.17
#6 0x080b9813 in scm_cell (car=19839, cdr=3084940304)
at /usr/include/libguile/inline.h:122
#7 0x0833729a in unpure_pure_container_pure_part (smob=0xb72b68e0)
at /home/mikesol/lilypond-git/lily/unpure-pure-container.cc:51
#8 0x0833803d in pure_mark (pure=0xb72b68e0)
at /home/mikesol/lilypond-git/lily/unpure-pure-container.cc:111
#9 0x0017c2d2 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#10 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#11 0x0017c438 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#12 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#13 0x0017c438 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#14 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#15 0x0017c29d in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#16 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#17 0x0017c369 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#18 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#19 0x0017c438 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#20 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#21 0x0017c29d in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#22 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#23 0x0017c369 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#24 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#25 0x0017c438 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#26 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#27 0x0017c369 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#28 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#29 0x0017c29d in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#30 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#31 0x0017c369 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#32 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#33 0x0017c29d in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#34 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#35 0x0017c369 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#36 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#37 0x0017c29d in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#38 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#39 0x0017c369 in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#40 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#41 0x0017c29d in scm_gc_mark_dependencies () from /usr/lib/libguile.so.17
#42 0x0017c50c in scm_gc_mark () from /usr/lib/libguile.so.17
#43 0x001d1805 in scm_threads_mark_stacks () from /usr/lib/libguile.so.17
#44 0x0017c5fa in scm_mark_all () from /usr/lib/libguile.so.17
#45 0x0017b60c in scm_i_gc () from /usr/lib/libguile.so.17
#46 0x0017b700 in scm_gc () from /usr/lib/libguile.so.17