mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 17:36:53 +01:00
Add per-row performance guidance
Document SetAutoCalcFields for per-row FlowFields and direct writes on iterated records, with focused reviewer retrieval cues. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 85be3fc4-5253-47b8-ba1b-6b8fd188fcea
This commit is contained in:
parent
9214f73819
commit
bad62d2763
7 changed files with 127 additions and 2 deletions
|
|
@ -0,0 +1,19 @@
|
||||||
|
codeunit 50493 "Perf Record Clone Bad"
|
||||||
|
{
|
||||||
|
procedure IncreaseCustomerCreditLimits(Percent: Decimal)
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
|
CustomerCopy: Record Customer;
|
||||||
|
begin
|
||||||
|
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||||
|
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||||
|
if Customer.FindSet(true) then
|
||||||
|
repeat
|
||||||
|
CustomerCopy.Copy(Customer);
|
||||||
|
CustomerCopy.Validate(
|
||||||
|
"Credit Limit (LCY)",
|
||||||
|
Round(CustomerCopy."Credit Limit (LCY)" * (1 + Percent / 100)));
|
||||||
|
CustomerCopy.Modify(true);
|
||||||
|
until Customer.Next() = 0;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,17 @@
|
||||||
|
codeunit 50492 "Perf Record Clone Good"
|
||||||
|
{
|
||||||
|
procedure IncreaseCustomerCreditLimits(Percent: Decimal)
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
|
begin
|
||||||
|
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||||
|
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||||
|
if Customer.FindSet(true) then
|
||||||
|
repeat
|
||||||
|
Customer.Validate(
|
||||||
|
"Credit Limit (LCY)",
|
||||||
|
Round(Customer."Credit Limit (LCY)" * (1 + Percent / 100)));
|
||||||
|
Customer.Modify(true);
|
||||||
|
until Customer.Next() = 0;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: performance
|
||||||
|
keywords: [clone, copy, recordref, gettable, by-value, modify, delete, loop]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Avoid cloning records before Modify or Delete in loops
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
Microsoft's [AL database-method performance guidance](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/optimize-sql-al-database-methods-and-performance-on-server#insert-modify-delete-and-locktable) states that cloning an iterated record before `Modify` or `Delete` restarts the SQL `SELECT` and issues an extra SQL statement for every row. The runtime treats `Record.Copy`, `RecordRef.GetTable`, and passing a record by value to a writing helper as clones in this situation.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Use `FindSet(true)` when the loop writes the traversed rows, and call `Modify` or `Delete` on that iterating record variable. If generic code is required, open and iterate the `RecordRef` directly instead of calling `GetTable` for each typed record. Keep a per-row loop when validation or row-specific behavior is required; this rule does not imply that `ModifyAll` or `DeleteAll` is equivalent.
|
||||||
|
|
||||||
|
See sample: `avoid-cloning-records-before-modify-delete-in-loops.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Inside an active traversal, copy the current row, convert it with `RecordRef.GetTable`, or pass it without `var` to a helper, then call `Modify` or `Delete` on that clone. Do not flag read-only snapshots, temporary records, or copies used to write a different target table; the documented extra-statement concern is clone-before-write on the traversed table.
|
||||||
|
|
||||||
|
See sample: `avoid-cloning-records-before-modify-delete-in-loops.bad.al`.
|
||||||
|
|
@ -0,0 +1,16 @@
|
||||||
|
codeunit 50491 "Perf AutoCalcFields Bad"
|
||||||
|
{
|
||||||
|
procedure CollectOverLimitCustomers(var CustomerNos: List of [Code[20]])
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
|
begin
|
||||||
|
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||||
|
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||||
|
if Customer.FindSet() then
|
||||||
|
repeat
|
||||||
|
Customer.CalcFields("Balance (LCY)");
|
||||||
|
if Customer."Balance (LCY)" > Customer."Credit Limit (LCY)" then
|
||||||
|
CustomerNos.Add(Customer."No.");
|
||||||
|
until Customer.Next() = 0;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,16 @@
|
||||||
|
codeunit 50490 "Perf AutoCalcFields Good"
|
||||||
|
{
|
||||||
|
procedure CollectOverLimitCustomers(var CustomerNos: List of [Code[20]])
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
|
begin
|
||||||
|
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||||
|
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||||
|
Customer.SetAutoCalcFields("Balance (LCY)");
|
||||||
|
if Customer.FindSet() then
|
||||||
|
repeat
|
||||||
|
if Customer."Balance (LCY)" > Customer."Credit Limit (LCY)" then
|
||||||
|
CustomerNos.Add(Customer."No.");
|
||||||
|
until Customer.Next() = 0;
|
||||||
|
end;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,26 @@
|
||||||
|
---
|
||||||
|
bc-version: [all]
|
||||||
|
domain: performance
|
||||||
|
keywords: [setautocalcfields, calcfields, calcsums, flowfield, loop, per-row]
|
||||||
|
technologies: [al]
|
||||||
|
countries: [w1]
|
||||||
|
application-area: [all]
|
||||||
|
---
|
||||||
|
|
||||||
|
# Use SetAutoCalcFields when each iterated row needs a FlowField
|
||||||
|
|
||||||
|
## Description
|
||||||
|
|
||||||
|
`Record.SetAutoCalcFields` has been available since runtime 1.0 and makes the specified FlowFields calculate as records are retrieved. Microsoft's [AL database-method performance guidance](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/optimize-sql-al-database-methods-and-performance-on-server#setautocalcfields) uses it to remove an explicit `CalcFields` call from every iteration when each row's FlowField drives a branch. This is different from `CalcSums`, which returns a total for the filtered set rather than a value for each row.
|
||||||
|
|
||||||
|
## Best Practice
|
||||||
|
|
||||||
|
Call `SetAutoCalcFields` before `FindSet` when every returned row needs the same FlowField for a comparison, branch, or per-record action. Use `CalcSums` instead when the required result is one aggregate over the filtered set (see `calcsums-instead-of-calcfields-in-loop.md`).
|
||||||
|
|
||||||
|
See sample: `use-setautocalcfields-for-per-row-flowfields.good.al`.
|
||||||
|
|
||||||
|
## Anti Pattern
|
||||||
|
|
||||||
|
Calling `CalcFields` inside the loop when every iteration reads the same FlowField. Each `CalcFields` request requires a separate SQL statement unless a compatible recent result is cached. Do not replace row-specific decisions with `CalcSums`; an aggregate cannot preserve which rows met the condition.
|
||||||
|
|
||||||
|
See sample: `use-setautocalcfields-for-per-row-flowfields.bad.al`.
|
||||||
|
|
@ -38,11 +38,16 @@ Discard files that are not applicable. Retain conditionally applicable files (an
|
||||||
Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
|
Narrow the relevant files to the subset that applies to the changes under review. For each relevant file, compute overlap against:
|
||||||
|
|
||||||
- The changed AL object names and types — especially tables, pages with SourceTable bindings, reports, queries, and codeunits performing record iteration.
|
- The changed AL object names and types — especially tables, pages with SourceTable bindings, reports, queries, and codeunits performing record iteration.
|
||||||
- The changed procedures and triggers, weighted toward those that perform loops, Find/FindSet/FindFirst calls, CalcFields, CalcSums, FlowField access, or cross-table navigation.
|
- The changed procedures and triggers, weighted toward those that perform loops, Find/FindSet/FindFirst calls, CalcFields, SetAutoCalcFields, CalcSums, FlowField access, record copying, RecordRef conversion, Modify/Delete calls, or cross-table navigation.
|
||||||
- Tokens extracted from the diff that relate to data access and hot-path costs (`SetRange`, `SetFilter`, `SetLoadFields`, `SetCurrentKey`, `FindSet`, `ReadIsolation`, `LockTable`, `ModifyAll`, `DeleteAll`, `TextBuilder`, `Dictionary`, `temporary`, `repeat`, `until`, `CalcFields`, `CalcSums`).
|
- Tokens extracted from the diff that relate to data access and hot-path costs (`SetRange`, `SetFilter`, `SetLoadFields`, `SetCurrentKey`, `FindSet`, `ReadIsolation`, `LockTable`, `ModifyAll`, `DeleteAll`, `Modify`, `Delete`, `Copy`, `RecordRef`, `GetTable`, `TextBuilder`, `Dictionary`, `temporary`, `repeat`, `until`, `CalcFields`, `SetAutoCalcFields`, `CalcSums`).
|
||||||
|
|
||||||
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
A file enters the candidate worklist when its `keywords` intersect the extracted tokens or its topic (derived from the index entry's `path`, `title`, and `description`) matches a changed object type. Read an article's full file — its `## Best Practice` / `## Anti Pattern` bodies — only after it makes the worklist; candidate selection uses the index alone.
|
||||||
|
|
||||||
|
Apply these targeted cues even when simple token overlap would rank the article below the worklist cutoff:
|
||||||
|
|
||||||
|
- Worklist `use-setautocalcfields-for-per-row-flowfields.md` when a record loop calls `CalcFields`, or when every row reads the same FlowField for a comparison, branch, or per-record action. Worklist `calcsums-instead-of-calcfields-in-loop.md` instead when the loop only accumulates one set total.
|
||||||
|
- Worklist `avoid-cloning-records-before-modify-delete-in-loops.md` when an iteration calls `Copy` or `RecordRef.GetTable` before `Modify`/`Delete`, or passes the iterated record without `var` to a helper that writes that record. Do not match a read-only copy, a temporary record, a different target table, or a `RecordRef` opened and iterated directly.
|
||||||
|
|
||||||
Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.
|
Once the candidate worklist is known, resolve layer-precedence conflicts per READ. Drop lower-precedence files whose normative guidance (`## Best Practice` or `## Anti Pattern`) directly contradicts a higher-precedence candidate, and record each dropped file in `suppressed` with `reason: "layer-precedence"`. Files that would have been candidates but are hidden because their layer is disabled in consumer configuration are recorded with `reason: "configuration"`. Files that never became candidates are NOT recorded in `suppressed`.
|
||||||
|
|
||||||
When the post-conflict worklist is empty because no applicable performance knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable performance knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
|
When the post-conflict worklist is empty because no applicable performance knowledge exists, or because configuration suppressed every candidate, emit `outcome: "no-knowledge"`. When the worklist is empty because no applicable performance knowledge matched the changes, emit `outcome: "completed"` with an empty `findings` array.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue