Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

perf: various minor perf fixes for ledger postings #26775

Merged
merged 15 commits into from
Aug 11, 2021

Conversation

ankush
Copy link
Member

@ankush ankush commented Aug 2, 2021

  • Ledger posting related code is on the "critical path" for most stock/accounting transactions as it generates ledger entries. This usually takes time roughly on the order of size of child tables.
  • Changed several methods to use db.value_cache for fetching details that won't change during a db transaction, this cache is dropped frequently when db instance dies, so it's safe to use.
  • reordered conditionals to avoid queries.

depends on frappe/frappe#13880

Outcome: Along with that ^ PR, ~20-25% saving in queries and time while submitting large documents with repeated validations. More stats in linked PR.

Note to reviewers: review commit by commit, would be easier that way.

@ankush ankush added backport version-13-hotfix needs-tests This PR needs automated unit-tests. labels Aug 2, 2021
erpnext/accounts/utils.py Outdated Show resolved Hide resolved
@ankush ankush force-pushed the minor_perf_fixes branch 2 times, most recently from 0c06bdc to e966c43 Compare August 6, 2021 10:09
@ankush ankush removed the needs-tests This PR needs automated unit-tests. label Aug 9, 2021
@ankush
Copy link
Member Author

ankush commented Aug 9, 2021

Dropped a refactor that would require additional tests for now, will add in separate PR (in next release)

@ankush ankush marked this pull request as ready for review August 9, 2021 05:53
@coveralls
Copy link

Coverage Status

Coverage increased (+0.002%) to 42.692% when pulling 966d643 on ankush:minor_perf_fixes into 7e0c57f on frappe:develop.

@marination
Copy link
Collaborator

Asset, POS and Attendance tests failing on other PRs as well. Not caused by this PR

@marination marination merged commit 9152715 into frappe:develop Aug 11, 2021
frappe-pr-bot pushed a commit to frappe-pr-bot/erpnext that referenced this pull request Aug 11, 2021
* perf: only validate if voucher is journal entry

* perf: optimize merge GLE

- Order fields such that comparison will fail faster
- Break out of loops if not matched

* perf: don't try to match SLE if count mismatch

* refactor: simplify initialize_previous_data

* perf: use cache for fetching valuation_method

These are set only once fields

* refactor: simplify get_future_stock_vouchers

* refactor: simplify get_voucherwise_gl_entries

* perf: fetch only required fields for GL comparison

`select *` fetches all fields, output of this function is only used for
comparing.

* perf: reorder conditions in PL cost center check

* perf: reduce query while validating new gle

* perf: use cache for validating warehouse props

These properties don't change often, no need to query everytime.

* perf: use cached stock settings to validate SLE

* docs: update misleading docstring

Co-authored-by: Marica <maricadsouza221197@gmail.com>
(cherry picked from commit 9152715)
ankush added a commit that referenced this pull request Aug 11, 2021
* perf: only validate if voucher is journal entry

* perf: optimize merge GLE

- Order fields such that comparison will fail faster
- Break out of loops if not matched

* perf: don't try to match SLE if count mismatch

* refactor: simplify initialize_previous_data

* perf: use cache for fetching valuation_method

These are set only once fields

* refactor: simplify get_future_stock_vouchers

* refactor: simplify get_voucherwise_gl_entries

* perf: fetch only required fields for GL comparison

`select *` fetches all fields, output of this function is only used for
comparing.

* perf: reorder conditions in PL cost center check

* perf: reduce query while validating new gle

* perf: use cache for validating warehouse props

These properties don't change often, no need to query everytime.

* perf: use cached stock settings to validate SLE

* docs: update misleading docstring

Co-authored-by: Marica <maricadsouza221197@gmail.com>
(cherry picked from commit 9152715)

Co-authored-by: Ankush <ankush@iwebnotes.com>
frappe-pr-bot pushed a commit to frappe-pr-bot/erpnext that referenced this pull request Aug 11, 2021
* perf: only validate if voucher is journal entry

* perf: optimize merge GLE

- Order fields such that comparison will fail faster
- Break out of loops if not matched

* perf: don't try to match SLE if count mismatch

* refactor: simplify initialize_previous_data

* perf: use cache for fetching valuation_method

These are set only once fields

* refactor: simplify get_future_stock_vouchers

* refactor: simplify get_voucherwise_gl_entries

* perf: fetch only required fields for GL comparison

`select *` fetches all fields, output of this function is only used for
comparing.

* perf: reorder conditions in PL cost center check

* perf: reduce query while validating new gle

* perf: use cache for validating warehouse props

These properties don't change often, no need to query everytime.

* perf: use cached stock settings to validate SLE

* docs: update misleading docstring

Co-authored-by: Marica <maricadsouza221197@gmail.com>
(cherry picked from commit 9152715)
@ankush ankush deleted the minor_perf_fixes branch August 31, 2021 13:52
asoral pushed a commit to asoral/erpnext that referenced this pull request Nov 12, 2021
* perf: only validate if voucher is journal entry

* perf: optimize merge GLE

- Order fields such that comparison will fail faster
- Break out of loops if not matched

* perf: don't try to match SLE if count mismatch

* refactor: simplify initialize_previous_data

* perf: use cache for fetching valuation_method

These are set only once fields

* refactor: simplify get_future_stock_vouchers

* refactor: simplify get_voucherwise_gl_entries

* perf: fetch only required fields for GL comparison

`select *` fetches all fields, output of this function is only used for
comparing.

* perf: reorder conditions in PL cost center check

* perf: reduce query while validating new gle

* perf: use cache for validating warehouse props

These properties don't change often, no need to query everytime.

* perf: use cached stock settings to validate SLE

* docs: update misleading docstring

Co-authored-by: Marica <maricadsouza221197@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants