Catalogue tool: extract --check finds the catalogue sets 29 times, and extract rewrites all 2,352 catalogues even when none changed #1169

Closed
opened 2026-10-31 03:04:29 +00:00 by tiagoagueda · 0 comments
Owner

This issue holds 2 findings of the audit that share one fix or the same files.

W1: Catalogue tool: extract --check walks the whole package 29 times and spends a third of its 110 s finding the catalogue sets again

What exists today

src/postulo/core/messages_tool.py:

  • catalogue_sets() (lines 211-233) runs sorted(project().package.rglob("locale")) over the whole package on every call. core_set() (236-237) calls it again, and so does sources() (385-402), which builds its nested list from a fresh catalogue_sets() call (line 392) for each of the 28 sets.
  • cmd_extract() (870-906) loops over catalogue_sets() and calls extract_all() per set, which calls sources(subject), which calls catalogue_sets() again: 29 calls per run.
  • Each walk enters every directory of src/postulo: 4,955 directories, of which 4,704 are inside a locale/ tree (84 languages times 28 sets, each with LC_MESSAGES). The SKIP_DIRS test is applied to the results after the walk, so it prunes nothing.

Measurement: uv run python -m cProfile -o … scripts/messages.py extract --check from the core checkout (one run, 1 min 54 s wall under the profiler, 110 s of profiled time, exit 0, nothing written to the repository; stats in the audit folder as work/w1_extract_check.prof). Cumulative times:

  • catalogue_sets: 29 calls, 33.4 s (28 of them from sources, 32.5 s). The scout's "21 s" was low.
  • sources own walk (rglob("*"), is_file, the _within test of every file against every nested root): about 8.6 s. The scout's "27 s for the sources walk into the locale dirs" was misattributed: the large cost is the repeated catalogue_sets walks, not the sources walk.
  • The rest is real work: extract_template 23.7 s, extract_python 19.5 s (tokenising 4.2 million tokens), parse of 2,352 catalogues 20.4 s, dump 12.1 s, merge 6.1 s.

extract --check runs in the CI Checks job (.forgejo/workflows/ci.yml:383) and in every plugin repository as postulo-messages extract --check. #724 fixed the same pattern inside build_report only (it calls catalogue_sets() once); the extract path was not touched. #1033 does not cover the tool's own speed.

What would change

  • Find the sets once per command: cmd_extract already has the list; pass the nested roots to sources() (or memoise catalogue_sets() per project(), cleared by use()).
  • Walk with pruning: Path.walk (Python 3.12+) or os.walk, removing SKIP_DIRS from the directory list in place, so neither walk enters the 4,704 locale/ directories, static/ or __pycache__.
  • Optional, separate: a ProcessPoolExecutor over the sets' extract_all, since the remaining cost is CPU-bound tokenising. Not needed for the main win.

No library replaces this; the fix uses the standard library's own walk.

What it saves or protects

medium: about 35 s of the 110 s extract --check (local, profiled), on every developer run and in the CI Checks job

Accessibility

none

Decisions

  • Memoise catalogue_sets() or pass the list down? Recommended: pass it down from the command (explicit, nothing to invalidate in tests that call use() on a temporary project), and prune the walk.
  • Fold W2 (skip writing unchanged catalogues) into the same PR? Recommended: yes, it is two lines in the same function.

W2: Catalogue tool: extract rewrites all 2,352 catalogues even when none changed

What exists today

src/postulo/core/messages_tool.py, cmd_extract() lines 886-893: in --check mode the current file is read and compared with the new text; without --check the comparison is skipped and path.write_text(text, …) (line 893) runs for every catalogue of every set: 84 languages times 28 sets, 2,352 files, 73 MB of .po (measured with du --apparent-size over every django.po under src). The content is usually identical, so git shows no change, but every modification time moves: git has to re-hash all of them on the next git status, editors with the files open reload them, and IDE indexers and file watchers fire 2,352 times. The same tool runs as postulo-messages extract in every plugin repository.

Not measured by running extract (forbidden here: it rewrites the tree); the write path is read from the code. The file is already read once for parse (line 884), so the comparison costs nothing extra.

What would change

Keep the text read at line 884 and write only when text != current; print how many catalogues were actually written. The --check and write branches then share one comparison.

What it saves or protects

small: 73 MB of file writes and 2,352 moved modification times saved on a run that changes nothing

Accessibility

none

Decisions

  • File on its own or with W1? Recommended: fold into W1's PR (same function, two lines); file separately only if W1 waits.

Found by the efficiency and reliability audit of 2026-10-30 (finding W1, W2; index postulo/postulo#1108). Checked against main at daee30b15 (2026-10-30).

This issue holds 2 findings of the audit that share one fix or the same files. ## W1: Catalogue tool: `extract --check` walks the whole package 29 times and spends a third of its 110 s finding the catalogue sets again ### What exists today `src/postulo/core/messages_tool.py`: - `catalogue_sets()` (lines 211-233) runs `sorted(project().package.rglob("locale"))` over the whole package on every call. `core_set()` (236-237) calls it again, and so does `sources()` (385-402), which builds its `nested` list from a fresh `catalogue_sets()` call (line 392) for each of the 28 sets. - `cmd_extract()` (870-906) loops over `catalogue_sets()` and calls `extract_all()` per set, which calls `sources(subject)`, which calls `catalogue_sets()` again: 29 calls per run. - Each walk enters every directory of `src/postulo`: 4,955 directories, of which 4,704 are inside a `locale/` tree (84 languages times 28 sets, each with `LC_MESSAGES`). The `SKIP_DIRS` test is applied to the results after the walk, so it prunes nothing. Measurement: `uv run python -m cProfile -o … scripts/messages.py extract --check` from the core checkout (one run, 1 min 54 s wall under the profiler, 110 s of profiled time, exit 0, nothing written to the repository; stats in the audit folder as `work/w1_extract_check.prof`). Cumulative times: - `catalogue_sets`: 29 calls, 33.4 s (28 of them from `sources`, 32.5 s). The scout's "21 s" was low. - `sources` own walk (`rglob("*")`, `is_file`, the `_within` test of every file against every nested root): about 8.6 s. The scout's "27 s for the sources walk into the locale dirs" was misattributed: the large cost is the repeated `catalogue_sets` walks, not the `sources` walk. - The rest is real work: `extract_template` 23.7 s, `extract_python` 19.5 s (tokenising 4.2 million tokens), `parse` of 2,352 catalogues 20.4 s, `dump` 12.1 s, `merge` 6.1 s. `extract --check` runs in the CI Checks job (`.forgejo/workflows/ci.yml:383`) and in every plugin repository as `postulo-messages extract --check`. #724 fixed the same pattern inside `build_report` only (it calls `catalogue_sets()` once); the extract path was not touched. #1033 does not cover the tool's own speed. ### What would change - Find the sets once per command: `cmd_extract` already has the list; pass the nested roots to `sources()` (or memoise `catalogue_sets()` per `project()`, cleared by `use()`). - Walk with pruning: `Path.walk` (Python 3.12+) or `os.walk`, removing `SKIP_DIRS` from the directory list in place, so neither walk enters the 4,704 `locale/` directories, `static/` or `__pycache__`. - Optional, separate: a `ProcessPoolExecutor` over the sets' `extract_all`, since the remaining cost is CPU-bound tokenising. Not needed for the main win. No library replaces this; the fix uses the standard library's own walk. ### What it saves or protects medium: about 35 s of the 110 s `extract --check` (local, profiled), on every developer run and in the CI Checks job ### Accessibility none ### Decisions - **Memoise `catalogue_sets()` or pass the list down?** Recommended: pass it down from the command (explicit, nothing to invalidate in tests that call `use()` on a temporary project), and prune the walk. - **Fold W2 (skip writing unchanged catalogues) into the same PR?** Recommended: yes, it is two lines in the same function. ## W2: Catalogue tool: `extract` rewrites all 2,352 catalogues even when none changed ### What exists today `src/postulo/core/messages_tool.py`, `cmd_extract()` lines 886-893: in `--check` mode the current file is read and compared with the new text; without `--check` the comparison is skipped and `path.write_text(text, …)` (line 893) runs for every catalogue of every set: 84 languages times 28 sets, 2,352 files, 73 MB of `.po` (measured with `du --apparent-size` over every `django.po` under `src`). The content is usually identical, so git shows no change, but every modification time moves: git has to re-hash all of them on the next `git status`, editors with the files open reload them, and IDE indexers and file watchers fire 2,352 times. The same tool runs as `postulo-messages extract` in every plugin repository. Not measured by running `extract` (forbidden here: it rewrites the tree); the write path is read from the code. The file is already read once for `parse` (line 884), so the comparison costs nothing extra. ### What would change Keep the text read at line 884 and write only when `text != current`; print how many catalogues were actually written. The `--check` and write branches then share one comparison. ### What it saves or protects small: 73 MB of file writes and 2,352 moved modification times saved on a run that changes nothing ### Accessibility none ### Decisions - **File on its own or with W1?** Recommended: fold into W1's PR (same function, two lines); file separately only if W1 waits. --- Found by the efficiency and reliability audit of 2026-10-30 (finding W1, W2; index postulo/postulo#1108). Checked against `main` at `daee30b15` (2026-10-30).
tiagoagueda added this to the 0.6.0 milestone 2026-10-31 03:04:29 +00:00
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
Postulo/postulo#1169
No description provided.