perf: improve column handling speed, fix infinite loops #1597
Conversation
The primary change here is that the column calculation algorithm now attempts to render columns as if they were the full remaining height on the page, rather than to render the entire content regardless of how long the page is. The worst-case behavior is effectively the same as that of the previous algorithm (as at least one additional pass would be required to determine how high balanced columns should be, but this applies in both cases), but if content would not fit on the page, this can bail and set the page immediately, rather than continuing significantly more calculations. As an associated benefit, this closes Kozea#1020, as we now handle multiple column breaks (which would force a page break) correctly. A test for this behavior is included.
That’s a nice improvement to avoid useless loops.
Thanks a lot for that 💜.
🎉
Probably. But I’ve been talking with a researcher during a W3C conference who was working on this topic, and he told me that it wasn’t a solved problem and that the "best" way to find column height had yet to be found.
That’s the biggest problem left in my opinion.
Sure.
We’ll keep this at least for version 55. Version 56 may include a lot of changes about the non-standard layouts, so… we’ll see! |
|
Excellent, thanks! I let some benchmarks run on a sample document overnight, just for reference:
I really should pull in the WeasyPerf stuff to at least benchmark those as well, though I don't know if this would touch them because they probably don't use |
The primary change here is that the column calculation algorithm now attempts to render columns as if they were the full remaining height on the page, rather than to render the entire content regardless of how long the page is.
The worst-case behavior is effectively the same as that of the previous algorithm (as at least one additional pass would be required to determine how high balanced columns should be, but this applies in both cases), but if content would not fit on the page, this can bail and set the page immediately, rather than continuing significantly more calculations.
As an associated benefit, this closes #1020, as we now handle multiple column breaks (which would force a page break) correctly. A test for this behavior is included.
In unscientific tests, this appears to be a significant speedup in worst-case column calculations, for example taking a 100+ page multi-column render from 45 minutes to under two minutes. A more realistic document render time was cut by about 50% (from a starting point of ~five minutes).
It is certainly plausible that there exist better algorithms for determining column heights (such as calculating possible breakpoints for children), but this provides significantly improved execution time over the current algorithm in both pathological and real-world tests, with minimal algorithmic changes. I certainly won't be offended if this gets entirely rewritten in the path to v55/56, but it might be nice to have in the meantime.