Skip to content

Conversation

@william-andre
Copy link
Contributor

@william-andre william-andre commented Oct 20, 2022

As the number of account.move, account.move.line,
account.bank.statement.line and account.journal is growing, the
accounting dashboard, which is the entry point of the app, gets slower
and slower.

There are multiple issues being addressed in this commit:

  • The data for each journal is computed journal by journal. This means
    that the number of queries run increases linearly with the number of
    journals. While the boilerplate around running multiple queries is
    negligible compared to the running time of the queries in this case,
    some queries take as much time to run for one journal or for many.
    To improve this, all the queries are now batched. This has been done
    by refactoring the code; all these functions are now called on as many
    records as needed1:
    • _get_journal_bank_account_balance
    • _get_journal_outstanding_payments_account_balance
    • _get_last_bank_statement
    • get_line_graph_datas
    • get_bar_graph_datas
    • get_journal_dashboard_datas
  • The gap detection and the entries' count are computed fields
    (has_sequence_holes and entries_count respectively). We don't need
    to display/compute these fields for all types of journals, but since
    they were mentioned by using a <field/> node in the view, they were
    computed for all journals displayed. Instead of using the <field/>
    node, we are now setting the value in the kanban_dashboard field.
  • Documents in foreign currencies on journals in foreign currencies need
    to get the rate in order to be aggregated in the journal's currency.
    There are 3 cases:
    • Document in journal's currency
    • Company's currency is the same as the journal's
    • Document, company and journal have 3 different currencies
      Before this commit, the second case will still fetch the daily rate
      for the document in order to do the conversion, but we actually
      already know the conversion; it is stored on the document.

Benchmark

On a populate database with:

  • 4 res.company (with accounting enabled)
  • 45 account.journal
  • 19k account.move
  • 140k account.move.line
  • 4k account.bank.statement.line
  • 4 account.bank.statement
Query count Query time Remaining time
Before fix 279 0.333s 0.375s
After fix (without update) 40 0.120s 0.170s
After fix (with update2) 38 0.100s 0.170s

Note that the currency conversion was disabled because the populate
database doesn't represent a realistic dataset regarding this. Disabling
it only improves the numbers before the fix.

Note

A lot of the time remaining comes from the aggregation of
draft/unpaid invoices with the correct rate done in python instead of in
SQL. This commit doesn't change the behavior but this could be rethought
from a function point of view.

Footnotes

  1. the old functions have been kept for compatibility, new ones are
    suffixed by _batched and made private if it wasn't the case.

  2. some optimization require the views and indexes to be updated

@robodoo
Copy link
Contributor

robodoo commented Oct 20, 2022

Pull request status dashboard

@C3POdoo C3POdoo added the RD research & development, internal work label Oct 20, 2022
@william-andre william-andre force-pushed the 16.0-dashboard-perf-wan branch 4 times, most recently from 430e56d to 7fc7b07 Compare October 25, 2022 09:00
@william-andre william-andre marked this pull request as ready for review October 25, 2022 09:01
@C3POdoo C3POdoo requested review from a team and qdp-odoo and removed request for a team October 25, 2022 09:02
@william-andre william-andre force-pushed the 16.0-dashboard-perf-wan branch 3 times, most recently from e580999 to 1378190 Compare October 25, 2022 12:33
@william-andre william-andre force-pushed the 16.0-dashboard-perf-wan branch from 1378190 to d329466 Compare November 25, 2022 12:52
Copy link
Contributor

@qdp-odoo qdp-odoo left a comment

Choose a reason for hiding this comment

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

It's a bit confused and I messed up. Or the contrary.

Multi currencies use case to check and discuss ☝️

curr_cache = {}
sale_purchase_journals._fill_dashboard_data_count(dashboard_data, 'account.move', 'entries_count', [])
for journal in sale_purchase_journals:
currency = journal.currency_id or journal.company_id.currency_id
Copy link
Contributor

@qdp-odoo qdp-odoo Nov 29, 2022

Choose a reason for hiding this comment

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

I believe that journal.currency_id doesn't have any sense here, although it appears (I check in Odoo 16) you can effectively set a secondary currency on a sale/purchase journal. 🙃
I don't think it has any interest, except maybe to default the secondary currency of invoices created in that journal, and reporting in this very dashboard... I saw it was like that before, but we should probably consider simplifying that, what do you think?

the proposed change doesn't alter the current behavior, so it's not a blocking point

@william-andre william-andre force-pushed the 16.0-dashboard-perf-wan branch from d329466 to a878af8 Compare February 21, 2023 17:22
@C3POdoo C3POdoo requested a review from a team February 21, 2023 17:24
@william-andre william-andre force-pushed the 16.0-dashboard-perf-wan branch 8 times, most recently from 703f67e to d6b1dcd Compare February 22, 2023 13:48
@william-andre william-andre force-pushed the 16.0-dashboard-perf-wan branch from d6b1dcd to 5cc7ed5 Compare March 8, 2023 13:25
@robodoo
Copy link
Contributor

robodoo commented Apr 4, 2023

@william-andre @qdp-odoo linked pull request(s) odoo/enterprise#33165 not ready. Linked PRs are not staged until all of them are ready.

robodoo pushed a commit that referenced this pull request Apr 6, 2023
`res.currency` already has some utilitises like `round` or
`compare_amounts` but until now we always needed to import
`format_amount` from the tools in order to format.

This commit unifies the API.

Part-of: #103697
@robodoo robodoo temporarily deployed to merge April 6, 2023 12:40 Inactive
@robodoo robodoo closed this in 0a38693 Apr 6, 2023
@robodoo robodoo closed this Apr 6, 2023
@fw-bot fw-bot deleted the 16.0-dashboard-perf-wan branch April 20, 2023 12:47
h4818 added a commit to odoo-dev/odoo that referenced this pull request Apr 24, 2023
Due to performance issues, the computation of the balance in GL was changed (odoo#103697)
Since it is computed in `_get_journal_dashboard_bank_running_balance` as the last statement balance (balance_end_real) + transactions (account.bank.statement.line since the last statement), we rename the label on the journal dashboard to "Running Balance".
Also, fix the order clause of the "Last Statement" balance as it has a problem when there are 2 statements on the same day.

OPW-3265511
robodoo pushed a commit that referenced this pull request Apr 25, 2023
Due to performance issues, the computation of the balance in GL was changed (#103697)
Since it is computed in `_get_journal_dashboard_bank_running_balance` as the last statement balance (balance_end_real) + transactions (account.bank.statement.line since the last statement), we rename the label on the journal dashboard to "Running Balance".
Also, fix the order clause of the "Last Statement" balance as it has a problem when there are 2 statements on the same day.

OPW-3265511

closes #119514

Related: odoo/enterprise#40194
Signed-off-by: William André (wan) <wan@odoo.com>
fw-bot pushed a commit to odoo-dev/odoo that referenced this pull request Apr 25, 2023
Due to performance issues, the computation of the balance in GL was changed (odoo#103697)
Since it is computed in `_get_journal_dashboard_bank_running_balance` as the last statement balance (balance_end_real) + transactions (account.bank.statement.line since the last statement), we rename the label on the journal dashboard to "Running Balance".
Also, fix the order clause of the "Last Statement" balance as it has a problem when there are 2 statements on the same day.

OPW-3265511

X-original-commit: 383f52a
fw-bot pushed a commit to odoo-dev/odoo that referenced this pull request Apr 25, 2023
Due to performance issues, the computation of the balance in GL was changed (odoo#103697)
Since it is computed in `_get_journal_dashboard_bank_running_balance` as the last statement balance (balance_end_real) + transactions (account.bank.statement.line since the last statement), we rename the label on the journal dashboard to "Running Balance".
Also, fix the order clause of the "Last Statement" balance as it has a problem when there are 2 statements on the same day.

OPW-3265511

X-original-commit: 383f52a
fw-bot pushed a commit to odoo-dev/odoo that referenced this pull request Apr 25, 2023
Due to performance issues, the computation of the balance in GL was changed (odoo#103697)
Since it is computed in `_get_journal_dashboard_bank_running_balance` as the last statement balance (balance_end_real) + transactions (account.bank.statement.line since the last statement), we rename the label on the journal dashboard to "Running Balance".
Also, fix the order clause of the "Last Statement" balance as it has a problem when there are 2 statements on the same day.

OPW-3265511

X-original-commit: 383f52a
robodoo pushed a commit that referenced this pull request Apr 26, 2023
Due to performance issues, the computation of the balance in GL was changed (#103697)
Since it is computed in `_get_journal_dashboard_bank_running_balance` as the last statement balance (balance_end_real) + transactions (account.bank.statement.line since the last statement), we rename the label on the journal dashboard to "Running Balance".
Also, fix the order clause of the "Last Statement" balance as it has a problem when there are 2 statements on the same day.

OPW-3265511

closes #119740

X-original-commit: 383f52a
Related: odoo/enterprise#40316
Signed-off-by: William André (wan) <wan@odoo.com>
Signed-off-by: Ayob Habib (ayh) <ayh@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 26, 2023
Due to performance issues, the computation of the balance in GL was changed (#103697)
Since it is computed in `_get_journal_dashboard_bank_running_balance` as the last statement balance (balance_end_real) + transactions (account.bank.statement.line since the last statement), we rename the label on the journal dashboard to "Running Balance".
Also, fix the order clause of the "Last Statement" balance as it has a problem when there are 2 statements on the same day.

OPW-3265511

closes #119752

X-original-commit: 383f52a
Related: odoo/enterprise#40328
Signed-off-by: William André (wan) <wan@odoo.com>
Signed-off-by: Ayob Habib (ayh) <ayh@odoo.com>
robodoo pushed a commit that referenced this pull request Apr 26, 2023
Due to performance issues, the computation of the balance in GL was changed (#103697)
Since it is computed in `_get_journal_dashboard_bank_running_balance` as the last statement balance (balance_end_real) + transactions (account.bank.statement.line since the last statement), we rename the label on the journal dashboard to "Running Balance".
Also, fix the order clause of the "Last Statement" balance as it has a problem when there are 2 statements on the same day.

OPW-3265511

closes #119764

X-original-commit: 383f52a
Related: odoo/enterprise#40334
Signed-off-by: William André (wan) <wan@odoo.com>
Signed-off-by: Ayob Habib (ayh) <ayh@odoo.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

RD research & development, internal work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants