From 0bb34a4a321bb47259d74e3d814a651251699aed Mon Sep 17 00:00:00 2001 From: Duncan Tourolle Date: Sat, 8 Aug 2026 14:27:20 +0200 Subject: [PATCH] perf(layout): cut page layout time by ~40% Layout was dominated by work that was either repeated per word or thrown away. Rendered output is unchanged: every page hashed byte-for-byte identical across 3 page sizes, 2 font scales, 2 font families, and 4 alignments x 4 column widths chosen to force heavy hyphenation. layout, 600x800, 40 pages ~200ms -> ~124ms layout, 1404x1872, 11 pages ~168ms -> ~111ms Measured, not guessed. The reflex fix - swapping list comprehensions for generators - measures slower here (602ns vs 425ns for the width sum), so those are left alone. - Line asked its font for the advance width of a space on every construction. FreeTypeFont.getlength(" ") costs ~18us, two orders of magnitude more than getmetrics(), and it landed once per line. Memoise per font object. - Line.add_word was quadratic in the words on a line. Each candidate word re-summed every width and rebuilt the whole per-gap spacing list, when fitting only ever reads the first gap. Gap spacings are now a plan materialised on demand (only render() reads the list), and widths come from a prefix sum. The prefix list, rather than one accumulator, is what keeps this exact: widths are floats, (total + w) - w need not give back total, and one ulp flips an overflow decision on a line that ends flush. - RenderingPosition.copy/__eq__/__hash__ all went through dataclasses.asdict, a deep recursive walk, over 8 immutable scalars. - paragraph_layouter built a Text per line purely to discard it. - AbstractStyle.__hash__ rebuilt a 15-tuple containing 5 enums on every dict lookup; memoised on the frozen instance (1037ns -> 160ns). - The pyphen dictionary wrapper was rebuilt for every word that overflowed its line, and word extraction stripped before splitting and tested each split result for emptiness, neither of which str.split() needs. Co-Authored-By: Claude Opus 5 --- pyWebLayout/abstract/inline.py | 16 +- pyWebLayout/concrete/text.py | 222 ++++++++++++++++------ pyWebLayout/io/readers/html_extraction.py | 14 +- pyWebLayout/layout/document_layouter.py | 9 +- pyWebLayout/layout/ereader_layout.py | 19 +- pyWebLayout/style/abstract_style.py | 14 +- 6 files changed, 222 insertions(+), 72 deletions(-) diff --git a/pyWebLayout/abstract/inline.py b/pyWebLayout/abstract/inline.py index e28761b..b934761 100644 --- a/pyWebLayout/abstract/inline.py +++ b/pyWebLayout/abstract/inline.py @@ -3,12 +3,25 @@ from pyWebLayout.core import Hierarchical from pyWebLayout.style import Font from pyWebLayout.style.abstract_style import AbstractStyle from typing import Tuple, Union, List, Optional, Dict, Any, Callable +from functools import lru_cache import pyphen # Import LinkType for type hints (imported at module level to avoid F821 linting error) from pyWebLayout.abstract.functional import LinkType +@lru_cache(maxsize=16) +def _hyphen_dict(language: Optional[str]) -> pyphen.Pyphen: + """ + The pyphen dictionary for a language, reused across words. + + Pyphen caches the parsed dictionary file itself, but rebuilding the wrapper + per word still costs about 40% of a hyphenation call, and hyphenation is + attempted for every word that overflows its line. + """ + return pyphen.Pyphen(lang=language) + + class Word: """ An abstract representation of a word in a document. Words can be split across @@ -186,8 +199,7 @@ class Word: bool: True if the word was hyphenated, False otherwise. """ - dic = pyphen.Pyphen(lang=self._style.language) - return list(dic.iterate(self._text)) + return list(_hyphen_dict(self._style.language).iterate(self._text)) ... diff --git a/pyWebLayout/concrete/text.py b/pyWebLayout/concrete/text.py index 8e00782..e3bbfa2 100644 --- a/pyWebLayout/concrete/text.py +++ b/pyWebLayout/concrete/text.py @@ -58,6 +58,15 @@ _width_cache: UsageCache = UsageCache(DEFAULT_WIDTH_CACHE_ENTRIES) _glyph_cache: SizedUsageCache = SizedUsageCache(DEFAULT_GLYPH_CACHE_BYTES, _glyph_entry_bytes) _glyph_subpixel_steps: int = DEFAULT_GLYPH_SUBPIXEL_STEPS +# Every Line asks its font for the advance width of a space. That single +# FreeTypeFont.getlength(" ") call costs ~18us -- two orders of magnitude more +# than getmetrics() -- because PIL shapes the string from scratch each time, and +# it lands once per line created, which dominates the cost of laying a line out. +# There are only ever a handful of distinct fonts in play, so memoise per font +# object. Values are wrapped in a 1-tuple because None is itself a legitimate +# result (fonts that cannot report a length) and must not read as a cache miss. +_space_advance_cache: Dict[Any, Tuple[Optional[int]]] = {} + # Set to False the first time the fast rasterisation path is found to be # unavailable (e.g. a PIL build without the private ImageDraw internals it uses), # after which every Text falls back to ImageDraw.text(). @@ -98,6 +107,35 @@ def clear_text_caches(): """Drop all cached widths and glyph bitmaps.""" _width_cache.clear() _glyph_cache.clear() + _space_advance_cache.clear() + + +def _space_advance(font) -> Optional[int]: + """ + The font's own advance width for a space, in whole pixels. + + None when the font cannot report one, which is the signal for callers to fall + back to their configured spacing range. + """ + try: + cached = _space_advance_cache.get(font) + except TypeError: + # Unhashable font object; measure without caching. + cached = None + else: + if cached is not None: + return cached[0] + + try: + value = int(round(font.getlength(" "))) + except (AttributeError, TypeError, ValueError): + value = None + + try: + _space_advance_cache[font] = (value,) + except TypeError: + pass + return value def text_cache_stats() -> Dict[str, Any]: @@ -215,7 +253,8 @@ class AlignmentHandler(ABC): def calculate_spacing_and_position(self, text_objects: List['Text'], available_width: int, min_spacing: int, max_spacing: int, - natural_spacing: Optional[int] = None + natural_spacing: Optional[int] = None, + total_width: Optional[float] = None ) -> Tuple[int, int, bool]: """ Calculate the spacing between words and starting position for the line. @@ -228,6 +267,11 @@ class AlignmentHandler(ABC): natural_spacing: The font's own space width. Ragged alignments use it as a constant gap; justification ignores it. Defaults to min_spacing when not supplied. + total_width: The summed width of `text_objects`, when the caller + already knows it. Purely an optimisation: a line asks its handler + to re-measure once per candidate word, and summing the whole line + each time makes filling a line quadratic in its word count. Omit + it and the sum is taken here as before. Returns: Tuple of (spacing_between_words, starting_x_position, overflow) @@ -242,7 +286,8 @@ class LeftAlignmentHandler(AlignmentHandler): available_width: int, min_spacing: int, max_spacing: int, - natural_spacing: Optional[int] = None + natural_spacing: Optional[int] = None, + total_width: Optional[float] = None ) -> Tuple[int, int, bool]: """ Calculate spacing and position for left-aligned text objects. @@ -269,7 +314,8 @@ class LeftAlignmentHandler(AlignmentHandler): spacing = min_spacing if natural_spacing is None else natural_spacing spacing = max(min_spacing, min(max_spacing, int(spacing))) - text_length = sum([text.width for text in text_objects]) + text_length = (sum([text.width for text in text_objects]) + if total_width is None else total_width) num_gaps = len(text_objects) - 1 # The spacing is constant whether or not the content fits: tightening a @@ -290,7 +336,8 @@ class CenterRightAlignmentHandler(AlignmentHandler): def calculate_spacing_and_position(self, text_objects: List['Text'], available_width: int, min_spacing: int, max_spacing: int, - natural_spacing: Optional[int] = None + natural_spacing: Optional[int] = None, + total_width: Optional[float] = None ) -> Tuple[int, int, bool]: """ Centre/right alignment: constant word space, line shifted as a block. @@ -300,7 +347,8 @@ class CenterRightAlignmentHandler(AlignmentHandler): the same spacing that will actually be used, so the line lands where it was measured to land. """ - word_length = sum([word.width for word in text_objects]) + word_length = (sum([word.width for word in text_objects]) + if total_width is None else total_width) # Handle single word case if len(text_objects) <= 1: @@ -329,13 +377,42 @@ class JustifyAlignmentHandler(AlignmentHandler): """Handler for justified text with full justification.""" def __init__(self): - # Store variable spacing for each gap to distribute remainder pixels - self._gap_spacings: List[int] = [] + # The per-gap spacings are described by a plan rather than stored outright, + # and materialised on demand by the _gap_spacings property below. Fitting a + # line calls this handler once per candidate word and only ever looks at the + # first gap; building the whole list on each of those probes made adding n + # words to a line O(n^2). Only render() reads the full list. + self._gap_uniform: Optional[int] = None + self._gap_residual: int = 0 + self._gap_count: int = 0 + self._gap_cache: Optional[List[int]] = [] + + @property + def _gap_spacings(self) -> List[int]: + """The spacing to apply at each gap, left to right.""" + if self._gap_cache is None: + if self._gap_uniform is not None: + self._gap_cache = [self._gap_uniform] * self._gap_count + else: + self._gap_cache = self._distribute(self._gap_residual, self._gap_count) + return self._gap_cache + + @staticmethod + def _distribute(total: int, num_gaps: int) -> List[int]: + """Split `total` pixels across `num_gaps` gaps by cumulative rounding.""" + gaps = [] + placed = 0 + for i in range(1, num_gaps + 1): + cumulative = int(round(total * i / num_gaps)) + gaps.append(cumulative - placed) + placed = cumulative + return gaps def calculate_spacing_and_position(self, text_objects: List['Text'], available_width: int, min_spacing: int, max_spacing: int, - natural_spacing: Optional[int] = None + natural_spacing: Optional[int] = None, + total_width: Optional[float] = None ) -> Tuple[int, int, bool]: """ Justified alignment distributes space to fill the entire line width. @@ -347,14 +424,17 @@ class JustifyAlignmentHandler(AlignmentHandler): is min_spacing to ensure readability. """ - word_length = sum([word.width for word in text_objects]) + word_length = (sum([word.width for word in text_objects]) + if total_width is None else total_width) residual_space = available_width - word_length num_gaps = max(1, len(text_objects) - 1) # Check if we have enough space for minimum spacing if residual_space // num_gaps < min_spacing: # Not enough space - this is overflow - self._gap_spacings = [min_spacing] * num_gaps + self._gap_uniform = min_spacing + self._gap_count = num_gaps + self._gap_cache = None return min_spacing, 0, True # Distribute the residual by cumulative rounding rather than by taking a @@ -365,14 +445,14 @@ class JustifyAlignmentHandler(AlignmentHandler): # ragged right edge on otherwise justified text. Rounding the running # total makes the gaps sum to the residual exactly. total = int(round(residual_space)) - self._gap_spacings = [] - placed = 0 - for i in range(1, num_gaps + 1): - cumulative = int(round(total * i / num_gaps)) - self._gap_spacings.append(cumulative - placed) - placed = cumulative + self._gap_uniform = None + self._gap_residual = total + self._gap_count = num_gaps + self._gap_cache = None - return self._gap_spacings[0], 0, False + # The first gap is the whole of the plan that fitting needs, and it falls + # out of the same cumulative rounding as _distribute would give it. + return int(round(total / num_gaps)), 0, False class Text(Renderable, Queriable): @@ -689,6 +769,9 @@ class Line(Box): """ super().__init__(origin, size, callback, sheet, mode, halign, valign) self._text_objects: List['Text'] = [] # Store Text objects directly + # Prefix sums of the widths in _text_objects, kept in step by _push_text / + # _pop_text. Element 0 is the empty sum. See _push_text for the rationale. + self._width_prefix: List[float] = [0.0] self._spacing = spacing # (min_spacing, max_spacing) self._font = font if font else Font() # Use default font if none provided self._current_width = 0 # Track the current width used @@ -704,10 +787,7 @@ class Line(Box): # The font's own space advance. Ragged alignments use this as their # constant word gap rather than stretching to fill the measure. - try: - self._natural_spacing = int(round(self._font.font.getlength(" "))) - except (AttributeError, TypeError, ValueError): - self._natural_spacing = None + self._natural_spacing = _space_advance(self._font.font) # Hyphenation configuration parameters self._min_word_length_for_brute_force = min_word_length_for_brute_force @@ -771,6 +851,45 @@ class Line(Box): """Set the next line in sequence""" self._next = line + @property + def _content_width(self) -> float: + """Summed width of the line's current contents.""" + return self._width_prefix[-1] + + def _push_text(self, text: 'Text'): + """ + Append a Text to the line, keeping the running width sum in step. + + Fitting a word is a trial: the candidate is pushed, measured, and popped + again if it did not fit, so the line's contents churn far more often than + they grow. Tracking the sum here rather than re-adding every width on each + measurement is what keeps filling a line linear in its word count. + + The sum is kept as a prefix list rather than as one accumulator that is + added to and subtracted from. Widths are floats, so `(total + w) - w` need + not give back `total` exactly, and a drift of one ulp is enough to flip an + overflow decision on a line that ends flush. Truncating a prefix list + restores the earlier total bit for bit, and each entry is built by the same + left-to-right addition sum() would perform. + """ + self._text_objects.append(text) + self._width_prefix.append(self._width_prefix[-1] + text.width) + + def _pop_text(self) -> 'Text': + """Remove the last Text from the line, keeping the width sum in step.""" + text = self._text_objects.pop() + self._width_prefix.pop() + return text + + def _measure(self, handler: Optional[AlignmentHandler] = None + ) -> Tuple[int, int, bool]: + """Ask an alignment handler to place the line's current contents.""" + if handler is None: + handler = self._alignment_handler + return handler.calculate_spacing_and_position( + self._text_objects, self._size[0], self._spacing[0], self._spacing[1], + self._natural_spacing, self._content_width) + def add_word(self, word: 'Word', part: Optional[Text] = None) -> Tuple[bool, @@ -789,7 +908,7 @@ class Line(Box): """ # First, add any pretext from previous hyphenation if part is not None: - self._text_objects.append(part) + self._push_text(part) self._words.append(word) part.add_line(self) @@ -818,10 +937,8 @@ class Line(Box): line=self) else: text = Text.from_word(word, self._draw) - self._text_objects.append(text) - spacing, position, overflow = self._alignment_handler.calculate_spacing_and_position( - self._text_objects, self._size[0], self._spacing[0], self._spacing[1], - self._natural_spacing) + self._push_text(text) + spacing, position, overflow = self._measure() if not overflow: # Word fits! Add it completely @@ -833,7 +950,7 @@ class Line(Box): return True, None # Word doesn't fit, remove it and try hyphenation - _ = self._text_objects.pop() + self._pop_text() # Step 1: Try pyphen hyphenation pyphen_splits = word.possible_hyphenation() @@ -866,11 +983,9 @@ class Line(Box): source=word) # Check if first part fits - self._text_objects.append(first_text) - spacing, position, overflow = self._alignment_handler.calculate_spacing_and_position( - self._text_objects, self._size[0], self._spacing[0], self._spacing[1], - self._natural_spacing) - _ = self._text_objects.pop() + self._push_text(first_text) + spacing, position, overflow = self._measure() + self._pop_text() if not overflow: # This split fits! Add it to valid options @@ -883,7 +998,7 @@ class Line(Box): first_text, second_text, spacing, position = best_split # Apply the split - self._text_objects.append(first_text) + self._push_text(first_text) first_text.line = self word.add_concete((first_text, second_text)) self._spacing_render = spacing @@ -894,7 +1009,7 @@ class Line(Box): # Step 3: Try brute force hyphenation (only for long words) if len(word.text) >= self._min_word_length_for_brute_force: # Calculate available space for the word - word_length = sum([text.width for text in self._text_objects]) + word_length = self._content_width spacing_length = self._spacing[0] * max(0, len(self._text_objects) - 1) remaining = self._size[0] - word_length - spacing_length @@ -938,10 +1053,8 @@ class Line(Box): source=word) # Verify the first part actually fits - self._text_objects.append(first_text) - spacing, position, overflow = self._alignment_handler.calculate_spacing_and_position( - self._text_objects, self._size[0], self._spacing[0], self._spacing[1], - self._natural_spacing) + self._push_text(first_text) + spacing, position, overflow = self._measure() if not overflow: # Brute force split works! @@ -954,7 +1067,7 @@ class Line(Box): return True, second_text else: # Doesn't fit, remove it - _ = self._text_objects.pop() + self._pop_text() # Step 4: Word cannot be hyphenated or split, move to next line return False, None @@ -972,9 +1085,7 @@ class Line(Box): # justified paragraph. handler = self.render_alignment_handler if len(self._text_objects) > 0: - spacing, position, overflow = handler.calculate_spacing_and_position( - self._text_objects, self._size[0], self._spacing[0], self._spacing[1], - self._natural_spacing) + spacing, position, overflow = self._measure(handler) self._spacing_render = spacing self._position_render = position @@ -982,28 +1093,33 @@ class Line(Box): # Start x_cursor at line origin plus any alignment offset x_cursor = self._origin[0] + self._position_render - for i, text in enumerate(self._text_objects): + + # Everything the loop needs that does not vary per word is resolved once. + # Only justified lines carry per-gap spacings; every other alignment uses + # the single spacing figured above. + texts = self._text_objects + last = len(texts) - 1 + draw = self._draw + default_spacing = self._spacing_render + gaps = handler._gap_spacings if isinstance(handler, JustifyAlignmentHandler) else () + gap_count = len(gaps) + + for i, text in enumerate(texts): # Update text draw context to current draw context - text._draw = self._draw + text._draw = draw text.set_origin(np.array([x_cursor, y_cursor])) # Determine next text object for continuous decoration - next_text = self._text_objects[i + 1] if i + \ - 1 < len(self._text_objects) else None + next_text = texts[i + 1] if i < last else None # Get the spacing for this specific gap (variable for justified text) - if isinstance(handler, JustifyAlignmentHandler) and \ - hasattr(handler, '_gap_spacings') and \ - i < len(handler._gap_spacings): - current_spacing = handler._gap_spacings[i] - else: - current_spacing = self._spacing_render + current_spacing = gaps[i] if i < gap_count else default_spacing # Render with next text information for continuous underline/strikethrough text.render(next_text, current_spacing) # Add text width, then spacing only if there are more words x_cursor += text.width - if i < len(self._text_objects) - 1: + if i < last: x_cursor += current_spacing def query_point(self, point: Tuple[int, int]) -> Optional['QueryResult']: diff --git a/pyWebLayout/io/readers/html_extraction.py b/pyWebLayout/io/readers/html_extraction.py index b962c4f..d29fc81 100644 --- a/pyWebLayout/io/readers/html_extraction.py +++ b/pyWebLayout/io/readers/html_extraction.py @@ -403,13 +403,13 @@ def extract_words_from_nodes(nodes: List, context: StyleContext) -> List[Word]: continue if isinstance(child, NavigableString): - # Plain text - split into words - text = str(child).strip() - if text: - word_texts = text.split() - for word_text in word_texts: - if word_text: - words.append(Word(word_text, context.font, context.background)) + # Plain text - split into words. Argument-less str.split() already + # discards surrounding whitespace and never yields an empty string, so + # it needs neither a preceding strip() nor a per-word emptiness test. + font = context.font + background = context.background + words.extend([Word(word_text, font, background) + for word_text in str(child).split()]) elif isinstance(child, Tag): # Special handling for tags (hyperlinks) if child.name.lower() == "a": diff --git a/pyWebLayout/layout/document_layouter.py b/pyWebLayout/layout/document_layouter.py index 3f19b2c..b3be244 100644 --- a/pyWebLayout/layout/document_layouter.py +++ b/pyWebLayout/layout/document_layouter.py @@ -163,12 +163,9 @@ def paragraph_layouter(paragraph: Paragraph, y_cursor = page._current_y_offset x_cursor = page.content_origin[0] - # Create a temporary Text object to calculate word width - if word: - temp_text = Text.from_word(word, page.measurement_draw) - temp_text.width - else: - pass + # `word` is accepted for call-site readability only: the line that is about + # to be created measures it when it is added, so measuring it here as well + # only paid for a Text object that was immediately discarded. return Line( spacing=word_spacing_constraints, diff --git a/pyWebLayout/layout/ereader_layout.py b/pyWebLayout/layout/ereader_layout.py index 8e50738..6d9f4f5 100644 --- a/pyWebLayout/layout/ereader_layout.py +++ b/pyWebLayout/layout/ereader_layout.py @@ -43,6 +43,19 @@ class RenderingPosition: remaining_pretext: Optional[str] = None # Hyphenated word continuation page_y_offset: int = 0 # Vertical position on page + def _key(self) -> Tuple[Any, ...]: + """ + The fields in declaration order. + + Copying, comparing and hashing a position all used to go through + dataclasses.asdict, which walks the field list and deep-copies each value. + Every field here is an immutable scalar, so that traversal bought nothing + and these three run constantly during page navigation and buffer lookups. + """ + return (self.chapter_index, self.block_index, self.word_index, + self.table_row, self.table_col, self.list_item_index, + self.remaining_pretext, self.page_y_offset) + def to_dict(self) -> Dict[str, Any]: """Serialize position for saving to file/database""" return asdict(self) @@ -54,17 +67,17 @@ class RenderingPosition: def copy(self) -> 'RenderingPosition': """Create a copy of this position""" - return RenderingPosition(**asdict(self)) + return RenderingPosition(*self._key()) def __eq__(self, other) -> bool: """Check if two positions are equal""" if not isinstance(other, RenderingPosition): return False - return asdict(self) == asdict(other) + return self._key() == other._key() def __hash__(self) -> int: """Make position hashable for use as dict key""" - return hash(tuple(asdict(self).values())) + return hash(self._key()) class ChapterInfo: diff --git a/pyWebLayout/style/abstract_style.py b/pyWebLayout/style/abstract_style.py index 30a9be2..9351425 100644 --- a/pyWebLayout/style/abstract_style.py +++ b/pyWebLayout/style/abstract_style.py @@ -112,7 +112,17 @@ class AbstractStyle: Since this is a frozen dataclass, it should be hashable by default, but we provide a custom implementation to ensure all fields are properly considered and to handle the Union types correctly. + + The result is memoised on first use. Styles are used as dictionary keys + throughout parsing and style resolution, and five of the fields are enum + members whose own __hash__ is a Python-level call, so rebuilding the + 15-tuple on every lookup was a measurable share of document parsing. The + class is frozen, so the value cannot go stale. """ + cached = self.__dict__.get('_hash_cache') + if cached is not None: + return cached + # Convert all values to hashable forms hashable_values = ( self.font_family, @@ -132,7 +142,9 @@ class AbstractStyle: self.parent_style_id ) - return hash(hashable_values) + result = hash(hashable_values) + object.__setattr__(self, '_hash_cache', result) + return result def merge_with(self, other: 'AbstractStyle') -> 'AbstractStyle': """