Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve runtime of time spanning functor #3587

Merged
merged 2 commits into from
Feb 2, 2024

Conversation

brdvd
Copy link
Contributor

@brdvd brdvd commented Jan 31, 2024

This PR started with the observation that on a large file (about 260 pages) the PrepareTimeSpanning was the slowest of all functors - which seemed counterintuitive. The PR improves the functor runtime, especially for larger files:

File size [number of pages] Functor runtime before [ms] Functor runtime after [ms]
10 18 3
266 2700 46

To somehow understand the runtime issue, consider the following example. There are two kind of spanning elements: those that live in one measure and those that extend over several measures.

TimeSpanning

With functor traversal we will visit start note, end note, tie for the green tie. This is good, since it means that by traversing backwards we can directly resolve endpoints. However, for the red slur we will visit start note, slur, end note. This is bad, since (independent of the traversal direction) we will always miss one endpoint in the first pass. So the slur must be kept in a list for the remaining pass and this is costly since each element in the list is ID compared against each layer element that is visited.

To improve the algorithm, we always pass forward, but collect all time spanning descendants at the begin of each measure. So both the red slur and green tie in the example above are collected before we visit its start and end note. This allows to resolve pretty much all time spanning elements in one go, if they are encoded in a sensible way (i.e. the element is encoded in the measure where it starts).

Open question: we still keep some code to resolve time spanning elements which are encoded outside of a measure (see VisitFloatingObject and VisitF). I simply did not know whether this is possible in MEI.

@lpugin lpugin merged commit 8964131 into rism-digital:develop Feb 2, 2024
5 checks passed
@lpugin
Copy link
Contributor

lpugin commented Feb 2, 2024

This is great! Many thanks.

@brdvd brdvd deleted the time-spanning branch February 3, 2024 22:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants