Skip to content

[16.0][PERF] l10n_br_account, l10n_br_sale: opt-in performance regression test framework#4741

Open
mileo wants to merge 17 commits into
OCA:16.0from
kmee:16.0-kmee-perf-framework
Open

[16.0][PERF] l10n_br_account, l10n_br_sale: opt-in performance regression test framework#4741
mileo wants to merge 17 commits into
OCA:16.0from
kmee:16.0-kmee-perf-framework

Conversation

@mileo

@mileo mileo commented Jul 20, 2026

Copy link
Copy Markdown
Member

This PR adds the performance regression test framework, kept fully opt-in so it costs nothing on the default CI. It is split from the optimization PRs (#4711 and the stacked series) on purpose, so those can be reviewed as pure code changes.

What it does

A PerfMixin that measures, for the BR invoice flow (Form path) and the SO to invoice flow (ORM path):

  • SQL query counts per step (deterministic), asserted against named limits;
  • a scaling-ratio guard (queries per line on 30 lines must stay under 1.5x the queries per line on 2 lines) that catches O(n^2) behaviour independently of the machine or the installed module set, so it never needs recalibration;
  • wall time, logged for reference, never asserted.

The limits are the regression guard: a change that reintroduces an O(n^2) path, a cache wipe, or a redundant tax-engine pass makes the suite fail.

How the opt-in works (tag technique)

Odoo's test runner only executes tests tagged standard by default, and the base test class adds that tag automatically. The perf classes are tagged to remove it:

@tagged("-standard", "post_install", "-at_install", "l10n_br_performance")

The -standard prefix takes the class out of the default run:

  1. Default CI (oca_run_tests, inv test) never runs them. oca_run_tests calls --test-enable without --test-tags, so -standard alone excludes the suite. Zero time added to every PR, and no query-count constant can break an unrelated build.
  2. Run them on demand, locally or on a dedicated job:
    odoo --test-enable --test-tags l10n_br_performance -u l10n_br_account,l10n_br_sale
    
  3. Run them on a specific PR from CI with one click, via a label-gated step (proposed addition to test.yml), so a reviewer can demand the proof on a performance-sensitive PR without changing anything for the others:
    on:
      pull_request:
        types: [opened, synchronize, reopened, labeled]
    ...
          - name: Run performance tests
            if: contains(github.event.pull_request.labels.*.name, 'performance')
            run: |
              unbuffer $(which odoo) -d ${PGDATABASE} \
                -u l10n_br_account,l10n_br_sale --test-enable \
                --test-tags l10n_br_performance --stop-after-init

CI time: before and after

Measured on the OCA CI (test with Odoo / test with OCB jobs):

test job wall-clock perf suite share
Suite in the default run (old #4711) ~30 min ~3.5 to 4 min (create_form ~52s each, cold and warm, plus the sale flow)
Opt-in (this PR, -standard) ~28 min (same as a no-perf PR) 0 min on default CI; the ~3.5 to 4 min only run when explicitly requested

So the suite adds nothing to routine PRs and stays one click (or one --test-tags) away when someone actually wants to measure.

Demo data / 18.0

These tests reuse AccountMoveBRCommon and its demo data. On 18.0 they would follow the demo-less test refactor already in progress there. This 16.0 version guards the current production series where the slowdowns are felt today.

Scope note (stacked diff)

This branch is stacked on the optimization series (#4711, #4716, #4717, #4727, #4721), because the query-count limits are calibrated against the optimized code. The GitHub diff therefore shows the whole series; only the last 6 commits are new here: the perf test suite, its CI calibration, the saved-document edition step, the P1b totals trigger, the opt-in tagging, and the label-gated CI step.

The label mechanism requires a maintainer to create the performance label in the repository. Once this PR is merged, adding that label to any PR runs the perf suite on it; nothing changes for PRs without the label. The step is already in this PR's test.yml, so it can be demonstrated live here: create the label, apply it to this PR, and the suite will run on the next CI trigger.

Update: moved into l10n_br_setup_tests (review follow-up)

Moved the performance test framework into l10n_br_setup_tests as suggested by @rvalyi, so it no longer sits in l10n_br_account/l10n_br_sale. Kept fully opt-in (-standard tag; runs only via the performance label or an explicit --test-tags l10n_br_performance). Validated at runtime: the two perf classes are discovered and pass from l10n_br_setup_tests (0 failed). Thanks for the review.

@OCA-git-bot

Copy link
Copy Markdown
Contributor

Hi @marcelsavegnago, @rvalyi, @renatonlima, @luismalta,
some modules you are maintaining are being modified, check this out!

@antoniospneto antoniospneto changed the title [16.0][ADD] l10n_br_account, l10n_br_sale: opt-in performance regression test framework [16.0][PERF] l10n_br_account, l10n_br_sale: opt-in performance regression test framework Jul 22, 2026
rvalyi
rvalyi previously approved these changes Jul 22, 2026

@rvalyi rvalyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think most of the comments I wrote in #4711 (review) still hold true:

  1. mostly the net benefit of this code to maintain is not going to be positive on 16.0 because the 16.0 branch will hard receive any big refactor anymore that would impact performance. And for the few ones like #4629, it's easy to check the native SQL query count in the logs to ensure there is no regression.
  2. also this is more code to maintain/migrate/sync in key modules like l10n_br_account and @mileo has a proven 10+ years track records of NOT MAINTAINING SHIT in OCA/l10n-brazil, not talking about his other masterpiece projects like erpbrasil that get almost no maintenance from Kmee either...

BTW "regressions" happened during the onchange to pre-compute conversion between August of 2025 and February 2026. But due to the changes in account, sale, purchase, stock modules we largely had to follow it, because native precompute fields would not trigger onchanges and values while editing would be inconsistent/unbalanced if we didn't follow that. But of course @mileo you missed this completely has you didn't help shit with that key topic during the last 3 years. I was okay to leave _compute_fiscal_fields as an onchange on 16.0 when I released it was had a performance impact but @antoniospneto mostly fixed it and insisted for the full conversion so we went for it. He also likely found a very big optimization here as he explained Engenere#62

But I would say, at the very least move this performance testing framework to the l10n_br_setup_tests module so it doesn't pollute l10n_br_account with unrelated things. I don't guarantee I'll accept it though, I didn't look enough yet, something to be expected when you open 30 AI PRs in 2 days, after you spent 4 years not maintaining either reviewing PR's for your own modules letting the others fix it entirely, see https://github.com/OCA/.github/blob/master/AI_POLICY.md

@CristianoMafraJunior CristianoMafraJunior left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remover o prefixo [16.0] dos commits

@rvalyi
rvalyi dismissed their stale review July 22, 2026 16:40

sorry, I clicked on the wrong the button, I meant "Changes Requested"

@rvalyi rvalyi left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, I clicked on the wrong button, I request changes; see my previous comments in #4741 (review)

Moreover, when I compare this and the 5-6 other AI PRs for performance from @mileo not saving a single minute in the 25+ minutes CI run to the work from @antoniospneto saving from 5 to 8 minutes here Engenere#62 it reminds me a meme:

Image

mileo added a commit to kmee/l10n-brazil that referenced this pull request Jul 23, 2026
… here (as suggested in review)

Move the l10n_br performance testing framework out of the business
modules and into l10n_br_setup_tests, so it no longer pollutes
l10n_br_account / l10n_br_sale with test infrastructure unrelated to
those modules' domain. Suggested by @rvalyi in the review of PR OCA#4741.

Moved (history preserved via git mv):
- l10n_br_account/tests/perf_common.py
    -> l10n_br_setup_tests/tests/perf_common.py
- l10n_br_account/tests/test_performance.py
    -> l10n_br_setup_tests/tests/test_performance_account.py
- l10n_br_sale/tests/test_performance.py
    -> l10n_br_setup_tests/tests/test_performance_sale.py

The moved suites still reuse l10n_br_account's AccountMoveBRCommon
(imported via odoo.addons.l10n_br_account.tests.common, which stays in
l10n_br_account) and now import PerfMixin from the local perf_common.
l10n_br_setup_tests gains l10n_br_account and l10n_br_sale as
dependencies so the common test class, the fiscal demo records and the
fiscal sale.order model are available.

Opt-in behaviour is unchanged: both suites keep the
"-standard"/"l10n_br_performance" tags, so they are excluded from the
default test run and executed only with
--test-tags l10n_br_performance.
@OCA-git-bot OCA-git-bot added mod:l10n_br_setup_tests Module l10n_br_setup_tests and removed mod:l10n_br_sale Module l10n_br_sale labels Jul 23, 2026
@mileo

mileo commented Jul 23, 2026

Copy link
Copy Markdown
Member Author

Sobre medir performance no CI: o tempo de execucao do runner varia demais (vimos uns 10 min entre runs do mesmo codigo), entao minuto de CI nao e uma metrica confiavel pra provar ganho - nem o nosso, nem o de ninguem. Por isso este framework asserta contagem de queries e uma razao de escala (queries/linha em 30 linhas <= 1.5x em 2 linhas, pega O(n^2)), que sao deterministicas e independem de maquina, e apenas loga o tempo, sem assert. E o sinal separado do ruido. Movido pra l10n_br_setup_tests como o @rvalyi sugeriu; opt-in (tag -standard), custo zero no CI default. Obrigado pela review.

mileo added 2 commits July 24, 2026 10:16
…group

`FiscalTax.account_taxes()` mapped each fiscal tax to its accounting taxes
with two searches per fiscal tax per line: one to resolve the accounting
tax group (`tax.group.account_tax_group()`) and one to fetch the
`account.tax` records of that group. On a document/order line with several
fiscal taxes this scaled the search count linearly with the number of taxes.

This reworks the batch entrypoint to:
- resolve every accounting tax group of the recordset in a single search
  (`_account_tax_group_map`), and
- fetch all `account.tax` of the involved groups in a single search,
  indexing them by group in memory,

then does the per-tax type/deductible filtering in Python. Semantics are
unchanged (same domain, same `limit=1` first-match on the group, same
filters). The per-record `_account_taxes()` helper is kept for
create/unlink and now shares the company-resolution helper.

Local benchmark (queries, warm=1, perf-oca), baseline -> after:
- create_form 2/30 lines: 732/7572 -> 672/6681
- so_create   2/30 lines: 116/360  -> 101/180
Other steps unchanged. Functional suite: 0 failed
(154 tests l10n_br_fiscal+account+sale).
…fuse ICMS searches

The fiscal tax mapping (`operation.line.map_fiscal_taxes`, which drives
`_compute_fiscal_tax_ids`) is a pure function of its input records and the
fiscal definition tables, yet it is re-run with identical inputs several
times per document line by the onchange/compute cascade -- and again on
every line that repeats a product. Each run issues the company/operation/
CFOP/partner tax-definition searches plus four separate ICMS-regulation
searches.

This adds:

1. A transaction-scoped cache (see new `fiscal_cache.py`). The cache dict is
   anchored on the database cursor, so its lifetime is exactly one
   transaction: every request/job gets a fresh empty cache and nothing leaks
   across transactions or workers (deliberately NOT a module-level dict).
   `map_fiscal_taxes` is split into a thin memoizing wrapper and the
   unchanged `_map_fiscal_taxes` body; the key encodes every input record id
   plus the scalar config fields the mapping branches on (tax framework,
   tax classification, states, ind_ie_dest, ICMS origin, ...).

2. `FiscalCacheMixin`, inherited by the fiscal *definition* models
   (tax.definition, icms.regulation, tax.classification, operation.line):
   any create/write/unlink on them wipes the transaction cache, so a
   definition edited mid-transaction (config screens, tests) forces a
   recompute. Documented limitation: an in-place edit of a non-keyed config
   field on a non-definition model (e.g. ncm.tax_ipi_id) during the same
   transaction that already mapped a line is not observed -- this does not
   happen in the supported document flows.

3. `icms.regulation.map_tax` now runs its four per-group ICMS definition
   searches (ICMS, ICMS-ST, ICMS-FCP for DIFAL, FCP-ST) as a SINGLE search
   over the OR of the per-group domains. Because each sub-domain pins a
   distinct tax_group_id, the combined result partitions cleanly by group and
   the per-group precedence (benefit > specific > generic, then ind_final),
   extracted into `_tax_definition_precedence`, is applied exactly as before.
   Append order matches the original call order so downstream last-wins
   resolution of `icms_tax_benefit_id` is unchanged.

Local benchmark (queries, warm=1, perf-oca), previous commit -> after:
- create_form 2/30 lines: 672/6681 -> 631/6052
- so_create   2/30 lines: 101/180  -> 89/120
Cumulative from wave-2 baseline: create_form 30 = 7572 -> 6052 (-20%);
so_create 30 = 360 -> 120 (-67%). Other steps unchanged.
Functional suite: 0 failed (154 tests l10n_br_fiscal+account+sale).
mileo added 15 commits July 24, 2026 10:16
`l10n_br_fiscal.tax.compute_taxes` is the tax-calculation core. Within a
single document save it is re-run with identical kwargs several times per
line (fiscal line precompute + document totals + account.tax hooks) and
again on every line that repeats the same product/price/quantity. The
computation is a pure function of its inputs, so this memoizes it per
transaction.

Mechanism:
- Reuses the transaction-scoped cache and FiscalCacheMixin invalidation
  introduced for the mapping memoization (fiscal_cache.py); l10n_br_fiscal.tax
  now inherits the mixin so an edited tax rate wipes the cache mid-transaction.
- `_compute_taxes_cache_key` builds a stable, hashable key: recordsets (incl.
  `self`) become sorted id tuples, scalars (floats used unrounded, as required
  for exact tax bases) are kept as-is. If any kwarg is of an unexpected type
  the key is `None` and memoization is skipped, so an unusual input can never
  cause a wrong cache hit.
- The result is never handed out by reference: callers mutate the per-tax
  dicts in place (e.g. `account.tax` flips `base` sign for negative bases), so
  both the hit and the first-miss paths return a copy via
  `_copy_compute_taxes_result` (copies the top-level and each per-domain dict;
  non-dict domain values, used as flags, are passed through untouched).

Local benchmark (queries, warm=1, perf-oca), previous commit -> after:
- create_form 2/30 lines: 631/6052 -> 616/5874
- post        2/30 lines:  70/78   ->  67/75
Cumulative from wave-2 baseline: create_form 30 = 7572 -> 5874 (-22.4%),
create_form 2 = 732 -> 616 (-15.8%), so_create 30 = 360 -> 120 (-67%),
post 78 -> 75. Functional suite: 0 failed (154 tests
l10n_br_fiscal+account+sale).
…savepoint rollback

The fiscal transaction cache is a per-cursor attribute that survives a
savepoint rollback (cr.clear() clears the ORM cache, not that attribute).
Assert that a tax rate edited and computed inside a rolled-back savepoint is
not served afterwards: the write_date-versioned (self-validating) cache key
makes the stale entry unreachable once the rollback reverts write_date.
…dant re-propagation)

Preserves the post-OCA#4072/OCA#4102/OCA#4088 architecture (no compute-in-compute, no
new precompute/depends) and attacks only the redundant re-propagation of the
tax fields.

Mechanism
---------
_compute_tax_fields persists its ~145 output fields with a single
line.update()/line.write(). In the Form onchange cascade this compute is
re-triggered many times per line (measured: ~37x/line, of which ~29 produce the
exact same null result) because the stored ``related`` fields (icms_cst_code,
ipi_cst_code, ...) get recomputed and read a source field produced by this
method, pulling the whole compute back in. On each re-trigger only the field
actually read was invalidated; the other ~144 are still cached with the right
value. Re-writing the full dict therefore fires modified() on every dependent
(the document monetary totals and those stored related fields) for values that
did not change, feeding the cascade; and on a persisted write() it makes the
write override re-sync fiscal_tax_ids and re-run the engine (the classic C1
double-pass).

This patch only propagates the fields whose cache value actually changes
(compared via convert_to_cache); fields absent from the cache were invalidated
and are always written, so no computed value is ever left stale. Fiscal
semantics are unchanged: the same final values land in every field, in both
draft (NewId/onchange) and persisted mode.

Measured (perf-oca, warm=1, base = 16.0-perf-wave3 @ 9d5a739)
----------------------------------------------------------------
- Cumulative CPU of _compute_tax_fields itself (account perf run, cold+warm):
  1.817s -> 1.099s (-40%). ~87% of the re-triggers now write nothing
  (2244 of ~2580 calls have an empty delta).
- SQL query counts unchanged (create_form 616/5874, post 67/75, so_create
  88/120, create_invoices 130/380, ...): the wave2 engine memoization already
  removed the double-pass SQL cost, so the residual create_form queries are
  Form-onchange reads over the wide fiscal schema (C4/C5), not writes. This
  patch is a pure Python/CPU win (the production bottleneck: ~58% Python),
  SQL-neutral, guards stay green.

Functional suite: 0 failed, 0 error(s) of 154 tests
(l10n_br_fiscal, l10n_br_account, l10n_br_sale).
The extra other_retentions_value field was added to the fiscal document
line form by overriding the deprecated fields_view_get method. The 16.0
web client loads views through get_views -> get_view -> _get_view
and never calls fields_view_get (which is itself only a reverse wrapper
around get_views), so the override was dead code: the field had silently
disappeared from the NFS-e line form.

On top of that, the override anchored on issqn_wh_value (a field that only
exists in the line form through the fiscal fields injection) inside a bare
try/except Exception that swallowed a missing anchor as a no-op, turning
any change to the injection contract into an invisible functional regression.

Move the field injection to the modern _get_view flow (runs right after the
mixin fiscal fields injection) and, when the anchor is missing, emit a
diagnosable warning instead of silently dropping the field. Also drops the
redundant second inject_fiscal_fields/postprocess_and_fields pass the
old override performed.
… injected fields

inject_fiscal_fields() hardcoded the triad of compute methods whose fields it
force-adds (invisible) to the business-document line views, and had no way for
a downstream module to influence which fields are injected.

Introduce two @api.model hooks on the fiscal line mixin, following the repo
pattern (_add_fields_to_amount, _get_amount_fields...):

* _fiscal_view_compute_methods(): the compute methods whose stored fields must
  stay available in the line views for the fiscal onchange protocol. Formalizes
  the previously hardcoded triad; extensible by submodules adding fiscal
  computes.
* _fiscal_view_pruned_fields(): stored fiscal computes that no line view reads,
  displays, edits or references in a modifier/domain (C4 census 'dead'). They
  are no longer injected into the invisible onchange collection; being
  compute+store, they are recomputed server-side on write, so dropping them from
  the view is value-preserving. Prunes ii_percent, simple_value,
  simple_without_icms_value, cofins_wh_base_type, pis_wh_base_type.

Extend the injection contract tests (l10n_br_purchase, l10n_br_sale_blanket_order
test_get_view) to assert both halves: fiscal drivers are injected, pruned dead
fields are not.
Add a per-tax value regression suite for DRAFT invoice-line editing through
a Form (the UI onchange cascade): overriding a per-tax selector
(icms/ipi/pis/cofins _tax_id), changing quantity/price_unit, and changing
fiscal_operation_line_id, asserting base/percent/value and CFOP recompute
with the true line inputs. Guards the OCA#4721 class of stale draft fiscal
value regressions. Assertions are relationship based (value == base * rate)
so they hold regardless of the company-specific mapped rates.
…ness from view payload)

The draft correctness of the per-line fiscal taxes depended on which computed
fields travelled in the line view/onchange payload (issue OCA#4721): reducing the
tree collection left ICMS stuck at the mapped 7% after the user selected 18%.

Root cause (instrumented on test_out_fiscal_invoice):
1. fiscal_tax_ids, cfop_id, ipi_guideline_id, tax_classification_id and
   icms_tax_benefit_id shared ONE multi-field editable compute. When the user
   overrides icms_tax_id, the onchange reconciles it into fiscal_tax_ids, but
   manually setting one output of a multi-field compute does NOT protect its
   siblings; reading a dropped sibling (ipi_guideline_id) re-ran the whole
   compute, re-deriving fiscal_tax_ids from the mapping and discarding the
   override.
2. _compute_product_fiscal_fields re-assigned product-derived inputs of the
   tax mapping (e.g. national_taxation_code_id) with unchanged values during
   the onchange cascade; each redundant assignment re-flagged fiscal_tax_ids
   for recompute, which then clobbered the override too.

Fix (non-breaking, schema and the 145 stored fields intact):
* Split the fiscal-operation mapping compute in two: _compute_fiscal_tax_ids
  now computes only fiscal_tax_ids, while cfop_id/ipi_guideline_id/
  tax_classification_id/icms_tax_benefit_id move to _compute_fiscal_operation_data.
  fiscal_tax_ids is therefore recomputed only when its real inputs change, not
  when a decoration field is read. map_fiscal_taxes is memoized per
  transaction, so the second call is virtually free.
* Delta-write _compute_product_fiscal_fields (mirrors the wave3 delta-write in
  _compute_tax_fields) so unchanged product fields no longer re-flag the tax
  mapping.

With the OCA#4721 tree-reduction stacked on top, test_out_fiscal_invoice and the
per-tax draft suite keep the correct values (ICMS 79.38 at 18%); the residual
tree-reduction test breakages are the deliberate editable-column removals of
that PR. Perf is query-neutral without the reduction (create_form 30:
5872 vs 5874q) and realizes the reduction gain with it (5571q, -5.2%).
…ge whitelist

Reduce the fiscal field collection injected into the business-document line
TREES (account.move, sale.order, purchase.order, stock, blanket orders) from
~240 to ~120 fields, keeping the full fiscal detail editable in the line form
dialog (the popup opened from the tree). In an editable o2m tree the inline
onchange payload is driven by the tree fields (not the line form dialog), so
every extra invisible field is fetched/computed on every onchange round, for
every line. Narrowing that collection shrinks the per-line payload while the
whole fiscal detail stays reachable in the popup (a separate datapoint).

The whitelist (``_fiscal_line_view_fields``) keeps the tax selector fields
(FISCAL_TAX_ID), every ``@api.onchange`` trigger and every stored non-computed
field; the modifier/domain/context closure of the injected nodes is force-kept
(``_fiscal_line_modifier_refs``) so a reduced tree can never raise "Unknown
field ... in modifier" at view load. The removed ~121 fields are all stored
computed fields that recompute server-side on save, hence value-preserving to
drop from the inline onchange collection.

Measured on the perf baseline (perf-oca, warm=1), stacked on P2(b):

    create_form 30 lines: 5874q / ~20.6s -> 5571q / ~13.7s (-5.2% queries,
                          ~-33% wall time)
    create_form 2 lines:   616q / ~1.29s ->  599q / ~1.07s

Prerequisite: PR OCA#4727 "deterministic draft recompute" (P2(b)). It decouples
draft fiscal correctness from the view payload: with the reduced onchange spec
a stored fiscal value (e.g. icms_value after switching icms_tax_id on a draft
line) is recomputed deterministically server-side instead of relying on the
full-payload onchange round-trip. Without OCA#4727 this reduction regressed the
draft ICMS rate; stacked on it, the value is correct.

The tests that edited the now-popup-only fiscal detail as inline tree columns
were migrated to preserve their intent (column -> popup / saved record):
* l10n_br_account TestMoveEdition.test_out_fiscal_invoice: the manual FCP
  override (icmsfcp_base/icmsfcp_value) is set through the line form dialog
  (popup) and asserted to persist.
* l10n_br_account TestPerTaxDraftEdition: the IPI/PIS/COFINS override cases
  select the tax live (the selector stays in the tree) and assert the per-tax
  value on the saved fiscal line -- the exact value OCA#4721 could leave stale;
  ICMS base/value/percent stay in the tree, so ICMS is still checked live.
Measure wall time and SQL query count of the BR fiscal flows, log them in a
stable greppable format (PERF l10n_br | ...) and guard against regressions:

* l10n_br_account: invoice via Form (UI path, all onchanges) with 2 and 30
  lines + action_post + draft unlink (registry cache clearing).
* l10n_br_sale: pure ORM path with 2 and 30 lines — create SO, confirm,
  _create_invoices, post.

Guards are query-count based (deterministic in CI) plus a queries-per-line
scaling ratio between the small and the large scenario, which catches O(n^2)
regressions regardless of machine speed. Wall time is always logged, never
asserted (absolute time limits are flaky on shared runners). Set
L10N_BR_PERF_STRICT=0 for log-only mode on databases with extra modules.

Baseline on OCA 16.0 @ 683173c (clean db):
  Form path:  create 2 lines = 975 queries/1.8s; 30 lines = 7709 queries/21.2s
  ORM path:   whole SO->invoice flow with 30 lines = 910 queries/1.5s
The Form/ORM asymmetry (21.2s vs 1.5s for the same 30-line invoice) isolates
the per-line onchange chain as the main UI bottleneck.
…A CI

The query-count ceilings were initially calibrated on a minimal database
(l10n_br_account + l10n_br_sale only). The OCA CI installs the whole
localization, which legitimately costs more queries: l10n_br_sale_stock
creates pickings on SO confirm (so_confirm: 14q -> 157q), payment/EDI
modules add triggers on invoice post (post: 70q -> 104q), etc.

Make the OCA CI environment the canonical calibration reference: limits =
CI measured (post-optimization, deterministic across the Odoo/OCB jobs,
spread <=1 query) * 1.15. Minimal-database runs stay far below the
ceilings, and the scaling-ratio guard — which catches O(n^2) behaviour —
is environment-independent and unchanged.

Both docstrings keep the minimal-db numbers for local reference.
The document monetary totals (~60 stored Monetary fields) are summed by
_compute_fiscal_amount from ~56 line amount fields, each exposed as an
o2m.<field> dependency. Every one is a cross-record (reverse relation) edge:
on the Form onchange cascade (where _compute_tax_fields rewrites those line
fields many times) and on every write of a saved document, all ~56 edges
re-flag the whole totals compute.

Add a single technical per-line aggregate, fiscal_amount_total_signal (Float,
stored), that depends on those 56 source line fields, and make the document
totals depend on o2m.fiscal_amount_total_signal instead of the 56 o2m.<field>
edges (57 -> 2 dependency edges). The signal only has to change when a source
changes, so the totals are re-triggered on the same events through one reverse
edge; _compute_fiscal_amount still sums the real line fields, so the column
values and schema are unchanged. A fallback keeps the explicit edges for line
models that do not carry the signal.

Query-neutral in the create_form benchmark (the gain is fewer modified()
propagations / cross-record edges, i.e. CPU on post/edition of saved
documents, which the query guard does not measure). Functional suites
(l10n_br_fiscal/account/sale) and perf guards stay green.
Add a measured ``edit_saved`` step to the BR invoice performance suite: it
bumps the quantity of every line of an already-SAVED draft invoice. Each write
re-flags the stored line amount fields; on flush they recompute and propagate
modified() to the ~60 document monetary totals -- the cross-record (reverse
o2m) path narrowed by the ``fiscal_amount_total_signal`` trigger. The existing
create_form/post steps do not cover this: there the lines are created, not
re-written after save, so the saved-document edition path had no guard.

The step's query count is deterministic and calibrated LOCALLY (perf-oca
warm=1 * 1.15; simple=147, complex=791). It is a NEW step with no OCA CI
reference yet, so it will be re-calibrated against the CI environment once it
lands there (CI installs the full localization and costs more queries), the
same way the create_form/post limits were.

Measured with and without P1(b) (the ``fiscal_amount_total_signal`` commit),
perf-oca, warm=1:

    step        with P1(b)          without P1(b)
    edit_saved  simple  127q ~0.07s   127q ~0.07s
    edit_saved  complex 687q ~1.18s   687q ~1.16s

Finding: the step is EXACTLY query-neutral -- 687q with and without the
trigger, deterministic across runs -- which confirms P1(b) is a query-neutral
change. Its actual gain is fewer modified() propagations / cross-record edges
(57 -> 2), a Python/CPU micro-cost that, on a single representative edition
pass, is negligible against the recompute+SQL work and stays below this
machine's wall-time variance (both scenarios overlap at ~1.1-1.4s). So the
value of edit_saved here is a query-regression guard for the saved-document
edition path; P1(b)'s CPU benefit accumulates under the heavy repeated line
rewrites of the Form onchange cascade (itself query-dominated) rather than on
a single saved-doc edit, and would need a profiler / quieter CI to surface as
wall time.
Tag the performance suites with `-standard` so they are excluded from the
default test run (`inv test` / OCA CI) and only execute when selected
explicitly via `--test-tags l10n_br_performance`. Keeps the framework out
of CI weight even when the branch is aggregated into a build.
The performance suite is tagged -standard, so the default CI never runs
it. This step lets any maintainer run it on a specific PR by adding the
"performance" label (one click), without affecting any other PR.
Requires a maintainer to create the "performance" label in the repo.
…(as suggested in review)

Move the l10n_br performance testing framework out of the business
modules and into l10n_br_setup_tests, so it no longer pollutes
l10n_br_account / l10n_br_sale with test infrastructure unrelated to
those modules' domain. Suggested by @rvalyi in the review of PR OCA#4741.

Moved (history preserved via git mv):
- l10n_br_account/tests/perf_common.py
    -> l10n_br_setup_tests/tests/perf_common.py
- l10n_br_account/tests/test_performance.py
    -> l10n_br_setup_tests/tests/test_performance_account.py
- l10n_br_sale/tests/test_performance.py
    -> l10n_br_setup_tests/tests/test_performance_sale.py

The moved suites still reuse l10n_br_account's AccountMoveBRCommon
(imported via odoo.addons.l10n_br_account.tests.common, which stays in
l10n_br_account) and now import PerfMixin from the local perf_common.
l10n_br_setup_tests gains l10n_br_account and l10n_br_sale as
dependencies so the common test class, the fiscal demo records and the
fiscal sale.order model are available.

Opt-in behaviour is unchanged: both suites keep the
"-standard"/"l10n_br_performance" tags, so they are excluded from the
default test run and executed only with
--test-tags l10n_br_performance.
@mileo
mileo force-pushed the 16.0-kmee-perf-framework branch from a872a75 to b19924d Compare July 24, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

mod:l10n_br_account Module l10n_br_account mod:l10n_br_fiscal Module l10n_br_fiscal mod:l10n_br_nfse Module l10n_br_nfse mod:l10n_br_purchase Module l10n_br_purchase mod:l10n_br_sale_blanket_order Module l10n_br_sale_blanket_order mod:l10n_br_setup_tests Module l10n_br_setup_tests series:16.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants