mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-08-06 09:26:52 +01:00
Correct performance knowledge guidance (#94)
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 667c64a8-eb36-4440-bc41-6a97d8fb5542 Co-authored-by: Jesper Schulz-Wedde <jesper.schulzwedde@microsoft.com>
This commit is contained in:
parent
5706959e4a
commit
0e06485027
31 changed files with 279 additions and 261 deletions
|
|
@ -2,13 +2,22 @@ report 50221 "Perf Sample AddLoadFields Bad"
|
||||||
{
|
{
|
||||||
dataset
|
dataset
|
||||||
{
|
{
|
||||||
// No AddLoadFields: every Cust. Ledger Entry column ships per row, even though
|
|
||||||
// only three columns feed the layout.
|
|
||||||
dataitem(CustLedgerEntry; "Cust. Ledger Entry")
|
dataitem(CustLedgerEntry; "Cust. Ledger Entry")
|
||||||
{
|
{
|
||||||
column(CustomerNo; "Customer No.") { }
|
column(CustomerNo; "Customer No.") { }
|
||||||
column(PostingDate; "Posting Date") { }
|
column(PostingDate; "Posting Date") { }
|
||||||
column(Amount; Amount) { }
|
column(Amount; Amount) { }
|
||||||
|
|
||||||
|
trigger OnAfterGetRecord()
|
||||||
|
begin
|
||||||
|
// Source Code is not a dataset column, so its first access causes a
|
||||||
|
// just-in-time load and updates the dataitem enumerator.
|
||||||
|
RegisterSourceCode("Source Code");
|
||||||
|
end;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
local procedure RegisterSourceCode(SourceCode: Code[10])
|
||||||
|
begin
|
||||||
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -10,8 +10,19 @@ report 50220 "Perf Sample AddLoadFields Good"
|
||||||
|
|
||||||
trigger OnPreDataItem()
|
trigger OnPreDataItem()
|
||||||
begin
|
begin
|
||||||
AddLoadFields("Customer No.", "Posting Date", Amount);
|
// Dataset columns are selected by the report compiler. Source Code is
|
||||||
|
// extra because only trigger code reads it.
|
||||||
|
CustLedgerEntry.AddLoadFields("Source Code");
|
||||||
|
end;
|
||||||
|
|
||||||
|
trigger OnAfterGetRecord()
|
||||||
|
begin
|
||||||
|
RegisterSourceCode("Source Code");
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
local procedure RegisterSourceCode(SourceCode: Code[10])
|
||||||
|
begin
|
||||||
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# In reports, declare the fields the layout needs with AddLoadFields
|
# Add trigger-only report fields in OnPreDataItem
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
Reports iterate dataitems on potentially large source tables and pipe rows into a layout. The partial-record optimization is the same idea as `use-setloadfields-for-partial-records.md`, but the API is different: per the upstream guidance, "for reports, use `AddLoadFields()` in `OnPreDataItem` trigger to add fields needed by the layout." `AddLoadFields` is additive — call it for each field the layout consumes — and runs once per dataitem before iteration begins.
|
Report dataitem field selection is calculated at compile time and once per dataitem type during execution. Fields referenced by dataset columns are selected automatically; fields used only in triggers are not. Use `AddLoadFields` in `OnPreDataItem` to supplement the automatic selection with normal fields that trigger code needs.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
In each dataitem's `OnPreDataItem` trigger, list the columns the layout binds to via `AddLoadFields(<field>, <field>, ...)`. The platform then materializes only those columns per row. Treat the layout column list as the spec: every column the layout uses must be added; columns the layout does not use should not be added.
|
When a dataitem trigger needs an extra field, add that field in `OnPreDataItem` before iteration starts. This supplements the compiler-selected fields and avoids the first just-in-time load and enumerator update when the trigger reads the extra field.
|
||||||
|
|
||||||
See sample: `addloadfields-in-report-onpredataitem.good.al`.
|
See sample: `addloadfields-in-report-onpredataitem.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Relying on the dataitem's default to load every field. On a report bound to a ledger-scale table this transfers an entire row per iteration, of which the layout reads a fraction.
|
Listing every dataset column in `AddLoadFields`, or omitting a known trigger-only field because the dataset already uses other fields. The former is redundant; the latter causes a just-in-time load on first access and can cause repeated loads when the record is copied or passed by value.
|
||||||
|
|
||||||
See sample: `addloadfields-in-report-onpredataitem.bad.al`.
|
See sample: `addloadfields-in-report-onpredataitem.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -1,21 +1,32 @@
|
||||||
codeunit 50128 "Perf Sample CommitInLoop Good"
|
codeunit 50128 "Perf Sample CommitInLoop Good"
|
||||||
{
|
{
|
||||||
procedure NormalizeCustomerNames()
|
procedure NormalizeCustomerNames()
|
||||||
|
var
|
||||||
|
LastCustomerNo: Code[20];
|
||||||
|
begin
|
||||||
|
// The outer loop owns checkpoints; the per-row loop contains no Commit.
|
||||||
|
while NormalizeNextChunk(LastCustomerNo, 500) do
|
||||||
|
Commit();
|
||||||
|
end;
|
||||||
|
|
||||||
|
local procedure NormalizeNextChunk(var LastCustomerNo: Code[20]; ChunkSize: Integer): Boolean
|
||||||
var
|
var
|
||||||
Customer: Record Customer;
|
Customer: Record Customer;
|
||||||
RowsInChunk: Integer;
|
RowsInChunk: Integer;
|
||||||
ChunkSize: Integer;
|
|
||||||
begin
|
begin
|
||||||
ChunkSize := 500;
|
Customer.SetCurrentKey("No.");
|
||||||
if Customer.FindSet(true) then
|
if LastCustomerNo <> '' then
|
||||||
|
Customer.SetFilter("No.", '>%1', LastCustomerNo);
|
||||||
|
if not Customer.FindSet(true) then
|
||||||
|
exit(false);
|
||||||
|
|
||||||
repeat
|
repeat
|
||||||
Customer.Name := UpperCase(Customer.Name);
|
Customer.Name := UpperCase(Customer.Name);
|
||||||
Customer.Modify();
|
Customer.Modify();
|
||||||
|
LastCustomerNo := Customer."No.";
|
||||||
RowsInChunk += 1;
|
RowsInChunk += 1;
|
||||||
if RowsInChunk >= ChunkSize then begin
|
until (RowsInChunk >= ChunkSize) or (Customer.Next() = 0);
|
||||||
Commit();
|
|
||||||
RowsInChunk := 0;
|
exit(true);
|
||||||
end;
|
|
||||||
until Customer.Next() = 0;
|
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -17,7 +17,7 @@ Commit ends the current write transaction. Calling it inside a per-row loop prod
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
If the batch is large enough that a single transaction is untenable, process it in checkpoints driven by an outer loop that each time picks up the next N rows. Commit once per checkpoint at a clearly defined safe boundary, not inside the per-row loop. Wrapping each chunk in `Codeunit.Run` gives the same effect with native rollback on failure — see `codeunit-run-as-atomic-sub-operation.md`.
|
If the batch is large enough that a single transaction is untenable, use an outer loop that selects and finishes the next N rows. Commit only after the inner row loop has returned and the checkpoint state identifies where the next chunk starts. A `Codeunit.Run` boundary can also own a chunk when its implicit commit and error behavior fit the caller — see `codeunit-run-as-atomic-sub-operation.md`.
|
||||||
|
|
||||||
See sample: `avoid-commit-inside-loops.good.al`.
|
See sample: `avoid-commit-inside-loops.good.al`.
|
||||||
|
|
||||||
|
|
@ -26,4 +26,3 @@ See sample: `avoid-commit-inside-loops.good.al`.
|
||||||
Placing Commit inside `repeat ... until Next() = 0` is almost always a mistake: it is unusual for the correctness of the operation to depend on per-row commits, and the cost of starting a new transaction on every row dominates the work.
|
Placing Commit inside `repeat ... until Next() = 0` is almost always a mistake: it is unusual for the correctness of the operation to depend on per-row commits, and the cost of starting a new transaction on every row dominates the work.
|
||||||
|
|
||||||
See sample: `avoid-commit-inside-loops.bad.al`.
|
See sample: `avoid-commit-inside-loops.bad.al`.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,13 +1,15 @@
|
||||||
codeunit 50253 "Perf Sample NPlus1 Bad"
|
codeunit 50253 "Perf Sample NPlus1 Bad"
|
||||||
{
|
{
|
||||||
procedure SumStdCost(var BOMLine: Record "BOM Component") TotalCost: Decimal
|
procedure SumStdCost(BOMNo: Code[20]; BOMVersionCode: Code[20]) TotalCost: Decimal
|
||||||
var
|
var
|
||||||
|
BOMLine: Record "Production BOM Line";
|
||||||
Item: Record Item;
|
Item: Record Item;
|
||||||
begin
|
begin
|
||||||
|
BOMLine.SetRange("Production BOM No.", BOMNo);
|
||||||
|
BOMLine.SetRange("Version Code", BOMVersionCode);
|
||||||
if BOMLine.FindSet() then
|
if BOMLine.FindSet() then
|
||||||
repeat
|
repeat
|
||||||
// Full-row Item.Get per BOM line — no partial loading, no caching.
|
if Item.Get(BOMLine."No.") then
|
||||||
Item.Get(BOMLine."No.");
|
|
||||||
if Item."Costing Method" = Item."Costing Method"::Standard then
|
if Item."Costing Method" = Item."Costing Method"::Standard then
|
||||||
TotalCost += Item."Standard Cost" * BOMLine."Quantity per";
|
TotalCost += Item."Standard Cost" * BOMLine."Quantity per";
|
||||||
until BOMLine.Next() = 0;
|
until BOMLine.Next() = 0;
|
||||||
|
|
|
||||||
|
|
@ -1,15 +1,38 @@
|
||||||
codeunit 50252 "Perf Sample NPlus1 Good"
|
query 50252 "Perf Sample BOM Cost"
|
||||||
{
|
{
|
||||||
procedure SumStdCost(var BOMLine: Record "BOM Component") TotalCost: Decimal
|
QueryType = Normal;
|
||||||
|
|
||||||
|
elements
|
||||||
|
{
|
||||||
|
dataitem(ProductionBOMLine; "Production BOM Line")
|
||||||
|
{
|
||||||
|
column(ProductionBOMNo; "Production BOM No.") { }
|
||||||
|
column(VersionCode; "Version Code") { }
|
||||||
|
column(QuantityPer; "Quantity per") { }
|
||||||
|
|
||||||
|
dataitem(Item; Item)
|
||||||
|
{
|
||||||
|
DataItemLink = "No." = ProductionBOMLine."No.";
|
||||||
|
DataItemTableFilter = "Costing Method" = const(Standard);
|
||||||
|
SqlJoinType = InnerJoin;
|
||||||
|
|
||||||
|
column(StandardCost; "Standard Cost") { }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50254 "Perf Sample NPlus1 Good"
|
||||||
|
{
|
||||||
|
procedure SumStdCost(BOMNo: Code[20]; BOMVersionCode: Code[20]) TotalCost: Decimal
|
||||||
var
|
var
|
||||||
Item: Record Item;
|
BOMCost: Query "Perf Sample BOM Cost";
|
||||||
begin
|
begin
|
||||||
Item.SetLoadFields("Costing Method", "Standard Cost");
|
BOMCost.SetRange(ProductionBOMNo, BOMNo);
|
||||||
if BOMLine.FindSet() then
|
BOMCost.SetRange(VersionCode, BOMVersionCode);
|
||||||
repeat
|
BOMCost.Open();
|
||||||
if Item.Get(BOMLine."No.") then
|
while BOMCost.Read() do
|
||||||
if Item."Costing Method" = Item."Costing Method"::Standard then
|
TotalCost += BOMCost.StandardCost * BOMCost.QuantityPer;
|
||||||
TotalCost += Item."Standard Cost" * BOMLine."Quantity per";
|
BOMCost.Close();
|
||||||
until BOMLine.Next() = 0;
|
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
A `Get` or `FindFirst` against a different record inside a loop body produces one database round-trip per iteration — the classic N+1 pattern. Per the upstream guidance, "Flag when a `Get()`/`FindFirst()` is called inside a loop for each record — this creates N+1 database round-trips." The cost only matters when the inner table is meaningful: lookups against temporary tables, singleton setup tables, enum-mapping tables, permission objects, or Role IDs are bounded and safe. The pattern to catch is the inner lookup that hits a production-scale table for every outer row.
|
A `Get` or `FindFirst` against another persistent table inside a loop can produce an N+1 access pattern: one outer query followed by repeated inner lookups. Server and primary-key caches can satisfy some `Get` calls, so a source-level `Get` is not proof of one SQL round-trip. The concern is an unbounded loop whose lookup keys are not known to repeat or remain cached.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
When the loop needs values from another record, lift the lookup out of the loop if the rows can be collected up front, or apply `SetLoadFields` so each inner read transfers only the columns the loop actually uses (see `use-setloadfields-for-partial-records.md`). When the inner record is small or bounded, leave the call site alone — the rule targets large-table inner lookups specifically.
|
Use a query object to join the outer and inner tables when the relationship and filters can be expressed as one query. If keys repeat, a dictionary cache can reduce lookups to one per distinct key. `SetLoadFields` can reduce the columns transferred by unavoidable inner reads, but it does not eliminate the N+1 shape and must not be presented as doing so.
|
||||||
|
|
||||||
See sample: `avoid-get-inside-loop-on-large-table.good.al`.
|
See sample: `avoid-get-inside-loop-on-large-table.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Iterating BOM lines and calling `Item.Get(BOMLine."No.")` per row to read a costing method, with no `SetLoadFields` on `Item`. Each iteration issues one query against Item (~800k rows) and pulls the entire row to read two fields. The fix is `Item.SetLoadFields("Costing Method", "Standard Cost");` ahead of the loop — still N reads, but each one transfers only the needed columns.
|
Iterating production BOM lines and calling `Item.Get(BOMLine."No.")` for each line when the same result can be produced by a query joining Production BOM Line to Item. Partial loading alone is only a payload mitigation for this pattern.
|
||||||
|
|
||||||
See sample: `avoid-get-inside-loop-on-large-table.bad.al`.
|
See sample: `avoid-get-inside-loop-on-large-table.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -13,11 +13,11 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
`MaintainSIFTIndex` on a key decides whether the SIFT aggregate structure is updated on every `INSERT`, `MODIFY`, and `DELETE` that touches the key's fields. With `Yes`, `CalcSums` and FlowField reads are immediate — but every write pays the cost of updating the aggregate. With `No`, writes are cheaper but the first aggregate read after a change has to rebuild. Neither value is universally correct; the right choice depends on how often the aggregate is read versus how often the underlying rows are written.
|
`MaintainSIFTIndex` on a key decides whether SQL Server maintains the SIFT indexed view as underlying rows change. With `Yes`, writes that affect the key or sum fields also maintain the indexed aggregate. With `No`, that SIFT indexed view is not maintained, so a compatible `CalcSums` or FlowField calculation is computed from the base table instead and may require scanning many rows. There is no deferred "first read rebuild" of the SIFT structure.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Measure read-to-write ratios for the key's SIFT fields under realistic workloads. Set `MaintainSIFTIndex = Yes` only on keys whose aggregates are read far more often than the rows are written (reporting keys on reference tables, dashboards). Set `No` on keys whose rows are written heavily and whose aggregates are read rarely (transactional ledger entries, import-staging tables).
|
Measure aggregate-read latency and write cost under realistic filters and volumes. Keep `MaintainSIFTIndex = true` when the maintained aggregate materially benefits frequent `CalcSums` or FlowField reads. Consider `false` when writes dominate and the less-frequent aggregate reads can tolerate calculation from the base table.
|
||||||
|
|
||||||
See sample: `choose-maintainsiftindex-by-read-write-ratio.good.al`.
|
See sample: `choose-maintainsiftindex-by-read-write-ratio.good.al`.
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -1,23 +1,30 @@
|
||||||
codeunit 50100 "Sales Document Processor"
|
codeunit 50100 "Sales Document Processor"
|
||||||
{
|
{
|
||||||
procedure ProcessDocument(var SalesHeader: Record "Sales Header")
|
procedure DescribeDocument(DocumentType: Enum "Sales Document Type"; DocumentNo: Code[20]): Text
|
||||||
|
var
|
||||||
|
SalesHeader: Record "Sales Header";
|
||||||
begin
|
begin
|
||||||
// Single top-level load pulls every field any branch might touch.
|
|
||||||
// Order records pay for Posting Date and Amount Including VAT that
|
|
||||||
// only the Invoice branch reads, and vice versa.
|
|
||||||
SalesHeader.SetLoadFields(
|
SalesHeader.SetLoadFields(
|
||||||
"Document Type", "No.", "Sell-to Customer No.",
|
"Sell-to Customer No.",
|
||||||
"Order Date", "Shipment Date", "Completely Shipped",
|
"Order Date", "Shipment Date", "Completely Shipped",
|
||||||
"Posting Date", "Amount Including VAT");
|
"Posting Date", "Due Date", "Payment Terms Code");
|
||||||
|
SalesHeader.Get(DocumentType, DocumentNo);
|
||||||
|
|
||||||
case SalesHeader."Document Type" of
|
case DocumentType of
|
||||||
SalesHeader."Document Type"::Order:
|
DocumentType::Order:
|
||||||
ProcessOrder(SalesHeader);
|
exit(DescribeOrder(SalesHeader));
|
||||||
SalesHeader."Document Type"::Invoice:
|
DocumentType::Invoice:
|
||||||
ProcessInvoice(SalesHeader);
|
exit(DescribeInvoice(SalesHeader));
|
||||||
end;
|
end;
|
||||||
end;
|
end;
|
||||||
|
|
||||||
local procedure ProcessOrder(var SalesHeader: Record "Sales Header") begin end;
|
local procedure DescribeOrder(SalesHeader: Record "Sales Header"): Text
|
||||||
local procedure ProcessInvoice(var SalesHeader: Record "Sales Header") begin end;
|
begin
|
||||||
|
exit(StrSubstNo('%1|%2|%3|%4', SalesHeader."Sell-to Customer No.", SalesHeader."Order Date", SalesHeader."Shipment Date", SalesHeader."Completely Shipped"));
|
||||||
|
end;
|
||||||
|
|
||||||
|
local procedure DescribeInvoice(SalesHeader: Record "Sales Header"): Text
|
||||||
|
begin
|
||||||
|
exit(StrSubstNo('%1|%2|%3|%4', SalesHeader."Sell-to Customer No.", SalesHeader."Posting Date", SalesHeader."Due Date", SalesHeader."Payment Terms Code"));
|
||||||
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,25 +1,34 @@
|
||||||
codeunit 50100 "Sales Document Processor"
|
codeunit 50100 "Sales Document Processor"
|
||||||
{
|
{
|
||||||
procedure ProcessDocument(var SalesHeader: Record "Sales Header")
|
procedure DescribeDocument(DocumentType: Enum "Sales Document Type"; DocumentNo: Code[20]): Text
|
||||||
|
var
|
||||||
|
SalesHeader: Record "Sales Header";
|
||||||
begin
|
begin
|
||||||
// Tier 1: the discriminator and any fields every branch reads.
|
SalesHeader.SetLoadFields("Sell-to Customer No.");
|
||||||
SalesHeader.SetLoadFields("Document Type", "No.", "Sell-to Customer No.");
|
|
||||||
|
|
||||||
case SalesHeader."Document Type" of
|
case DocumentType of
|
||||||
SalesHeader."Document Type"::Order:
|
DocumentType::Order:
|
||||||
begin
|
begin
|
||||||
// Tier 2: extend the load only on the branch that needs these fields.
|
SalesHeader.AddLoadFields("Order Date", "Shipment Date", "Completely Shipped");
|
||||||
SalesHeader.SetLoadFields("Order Date", "Shipment Date", "Completely Shipped");
|
SalesHeader.Get(DocumentType, DocumentNo);
|
||||||
ProcessOrder(SalesHeader);
|
exit(DescribeOrder(SalesHeader));
|
||||||
end;
|
end;
|
||||||
SalesHeader."Document Type"::Invoice:
|
DocumentType::Invoice:
|
||||||
begin
|
begin
|
||||||
SalesHeader.SetLoadFields("Posting Date", "Amount Including VAT");
|
SalesHeader.AddLoadFields("Posting Date", "Due Date", "Payment Terms Code");
|
||||||
ProcessInvoice(SalesHeader);
|
SalesHeader.Get(DocumentType, DocumentNo);
|
||||||
|
exit(DescribeInvoice(SalesHeader));
|
||||||
end;
|
end;
|
||||||
end;
|
end;
|
||||||
end;
|
end;
|
||||||
|
|
||||||
local procedure ProcessOrder(var SalesHeader: Record "Sales Header") begin end;
|
local procedure DescribeOrder(SalesHeader: Record "Sales Header"): Text
|
||||||
local procedure ProcessInvoice(var SalesHeader: Record "Sales Header") begin end;
|
begin
|
||||||
|
exit(StrSubstNo('%1|%2|%3|%4', SalesHeader."Sell-to Customer No.", SalesHeader."Order Date", SalesHeader."Shipment Date", SalesHeader."Completely Shipped"));
|
||||||
|
end;
|
||||||
|
|
||||||
|
local procedure DescribeInvoice(SalesHeader: Record "Sales Header"): Text
|
||||||
|
begin
|
||||||
|
exit(StrSubstNo('%1|%2|%3|%4', SalesHeader."Sell-to Customer No.", SalesHeader."Posting Date", SalesHeader."Due Date", SalesHeader."Payment Terms Code"));
|
||||||
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -13,16 +13,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
When record processing branches on state, different branches typically read different fields. A single `SetLoadFields` at the top listing every field any branch might touch pulls more data than any individual execution path needs — on the hot path, the rest is loaded for nothing. A two-tier approach matches loading to actual usage: load the fields the `case` expression evaluates plus any fields every branch uses, then add a branch-local `SetLoadFields` inside each branch for that branch's extra fields.
|
When a known input determines which fields a subsequent record read will use, a single `SetLoadFields` containing every branch's fields loads unnecessary columns. Build the selection before `Get`, `FindFirst`, or `FindSet`: use `SetLoadFields` for fields common to every branch, then `AddLoadFields` for the selected branch. `SetLoadFields` replaces the current selection, while `AddLoadFields` preserves it.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Before the `case`, call `SetLoadFields` with the minimal set — the discriminator field and fields common to every branch. Inside each branch, before the first access to a branch-specific field, add a second `SetLoadFields` covering those fields. The platform honors the in-branch call for the next record operation, so the extra data is fetched only when the branch runs.
|
Call `SetLoadFields` with the common fields. In each branch, call `AddLoadFields` with that branch's normal fields and then perform the record read. This applies only when the discriminator is known before the read; branching on a field from an already-loaded row is too late to tailor that row's initial SQL projection.
|
||||||
|
|
||||||
See sample: `load-common-fields-before-branching-on-case.good.al`.
|
See sample: `load-common-fields-before-branching-on-case.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
A single top-level `SetLoadFields` enumerating every field any branch might read. On records whose state routes them to the fast common branch, the rarely-needed fields are still loaded — the optimization becomes a net-neutral or net-negative change on the hot path.
|
A single top-level `SetLoadFields` enumerating every branch's fields, or a branch-local `SetLoadFields` that accidentally discards the common selection. Both make the declared load plan differ from the fields the selected path actually uses.
|
||||||
|
|
||||||
See sample: `load-common-fields-before-branching-on-case.bad.al`.
|
See sample: `load-common-fields-before-branching-on-case.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -1,24 +0,0 @@
|
||||||
codeunit 50100 "Recent Orders Summary"
|
|
||||||
{
|
|
||||||
procedure SummarizeRecentOrders(StartDate: Date; EndDate: Date)
|
|
||||||
var
|
|
||||||
SalesHeader: Record "Sales Header";
|
|
||||||
begin
|
|
||||||
// "Document Type" and "Document Date" are listed in SetLoadFields even
|
|
||||||
// though they appear only in filters. Per-row values are transferred
|
|
||||||
// for columns the processing body never reads.
|
|
||||||
SalesHeader.SetLoadFields(
|
|
||||||
"Document Type", "Document Date",
|
|
||||||
"No.", "Sell-to Customer No.", "Amount Including VAT");
|
|
||||||
|
|
||||||
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Order);
|
|
||||||
SalesHeader.SetRange("Document Date", StartDate, EndDate);
|
|
||||||
|
|
||||||
if SalesHeader.FindSet() then
|
|
||||||
repeat
|
|
||||||
Emit(SalesHeader."No.", SalesHeader."Sell-to Customer No.", SalesHeader."Amount Including VAT");
|
|
||||||
until SalesHeader.Next() = 0;
|
|
||||||
end;
|
|
||||||
|
|
||||||
local procedure Emit(No: Code[20]; CustNo: Code[20]; Amount: Decimal) begin end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,22 +0,0 @@
|
||||||
codeunit 50100 "Recent Orders Summary"
|
|
||||||
{
|
|
||||||
procedure SummarizeRecentOrders(StartDate: Date; EndDate: Date)
|
|
||||||
var
|
|
||||||
SalesHeader: Record "Sales Header";
|
|
||||||
begin
|
|
||||||
// "Document Type" and "Document Date" are used only in the filters below.
|
|
||||||
// The database index handles them; there is no need to load their values
|
|
||||||
// into AL memory for every row.
|
|
||||||
SalesHeader.SetLoadFields("No.", "Sell-to Customer No.", "Amount Including VAT");
|
|
||||||
|
|
||||||
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Order);
|
|
||||||
SalesHeader.SetRange("Document Date", StartDate, EndDate);
|
|
||||||
|
|
||||||
if SalesHeader.FindSet() then
|
|
||||||
repeat
|
|
||||||
Emit(SalesHeader."No.", SalesHeader."Sell-to Customer No.", SalesHeader."Amount Including VAT");
|
|
||||||
until SalesHeader.Next() = 0;
|
|
||||||
end;
|
|
||||||
|
|
||||||
local procedure Emit(No: Code[20]; CustNo: Code[20]; Amount: Decimal) begin end;
|
|
||||||
}
|
|
||||||
|
|
@ -1,28 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: performance
|
|
||||||
keywords: [setloadfields, filter, field-exclusion, index]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Omit filter-only fields from SetLoadFields
|
|
||||||
|
|
||||||
> Contributions welcome — open a PR to refine or extend this article.
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
Fields used only in `SetRange` and `SetFilter` do their work at the database level using indexes; their values never need to be loaded into AL memory for the filter to apply. Listing such fields in `SetLoadFields` costs the transfer and memory footprint of every row's value for no functional benefit. Distinguishing filter-only fields from processing fields keeps the loaded column set as narrow as the iterating code actually reads.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
Include in `SetLoadFields` exactly the fields the iterating code reads. Fields referenced only in `SetRange`/`SetFilter` stay out of the list — filtering continues to work correctly because the database uses the index. Treat the audit as "what does the `repeat…until` block touch?" rather than "what does this procedure mention?".
|
|
||||||
|
|
||||||
See sample: `omit-filter-only-fields-from-setloadfields.good.al`.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Listing every field the procedure mentions in `SetLoadFields`, including date-range or status fields that appear only in filters. The loaded record now carries per-row values for columns the processing body never reads, inflating memory and network cost without changing any behavior.
|
|
||||||
|
|
||||||
See sample: `omit-filter-only-fields-from-setloadfields.bad.al`.
|
|
||||||
|
|
@ -2,8 +2,7 @@ codeunit 50100 "Document Router"
|
||||||
{
|
{
|
||||||
procedure Route(SalesHeader: Record "Sales Header")
|
procedure Route(SalesHeader: Record "Sales Header")
|
||||||
begin
|
begin
|
||||||
// In this deployment Orders are ~85% of posting calls, Invoices ~12%,
|
// Profiling shows Orders are the common case, so that branch goes first.
|
||||||
// and the rest are edge cases. The hot branch goes first.
|
|
||||||
case SalesHeader."Document Type" of
|
case SalesHeader."Document Type" of
|
||||||
SalesHeader."Document Type"::Order:
|
SalesHeader."Document Type"::Order:
|
||||||
RouteOrder(SalesHeader);
|
RouteOrder(SalesHeader);
|
||||||
|
|
@ -11,15 +10,16 @@ codeunit 50100 "Document Router"
|
||||||
RouteInvoice(SalesHeader);
|
RouteInvoice(SalesHeader);
|
||||||
SalesHeader."Document Type"::"Credit Memo":
|
SalesHeader."Document Type"::"Credit Memo":
|
||||||
RouteCreditMemo(SalesHeader);
|
RouteCreditMemo(SalesHeader);
|
||||||
|
SalesHeader."Document Type"::Quote:
|
||||||
|
RouteQuote(SalesHeader);
|
||||||
SalesHeader."Document Type"::"Return Order":
|
SalesHeader."Document Type"::"Return Order":
|
||||||
RouteReturnOrder(SalesHeader);
|
RouteReturnOrder(SalesHeader);
|
||||||
else
|
|
||||||
Error('Unexpected document type %1', SalesHeader."Document Type");
|
|
||||||
end;
|
end;
|
||||||
end;
|
end;
|
||||||
|
|
||||||
local procedure RouteOrder(SalesHeader: Record "Sales Header") begin end;
|
local procedure RouteOrder(SalesHeader: Record "Sales Header") begin end;
|
||||||
local procedure RouteInvoice(SalesHeader: Record "Sales Header") begin end;
|
local procedure RouteInvoice(SalesHeader: Record "Sales Header") begin end;
|
||||||
|
local procedure RouteQuote(SalesHeader: Record "Sales Header") begin end;
|
||||||
local procedure RouteCreditMemo(SalesHeader: Record "Sales Header") begin end;
|
local procedure RouteCreditMemo(SalesHeader: Record "Sales Header") begin end;
|
||||||
local procedure RouteReturnOrder(SalesHeader: Record "Sales Header") begin end;
|
local procedure RouteReturnOrder(SalesHeader: Record "Sales Header") begin end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -13,16 +13,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
The AL `case` statement evaluates branches in the order they appear. When the distribution of the discriminator is heavily skewed — one or two values handle the vast majority of records, and the rest handle edge cases — the average cost of the statement is dominated by how many branches precede the common one. For evenly distributed discriminators the order does not matter; for skewed distributions it changes the hot-path cost of every call site.
|
AL documentation does not guarantee that a `case` statement uses a linear comparison strategy, so branch frequency alone is not proof of a performance issue. Reordering is justified only when profiling on the target runtime shows that a large, heavily skewed `case` is a material hot path. It is not a default review finding.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Where the runtime frequency of values is known or measurable, list the common branches first. An `else` arm that handles unexpected values belongs last. When the common branch is also the simplest to evaluate, the placement compounds: the hot path is both short and cheap, and the uncommon branches are never touched on typical records.
|
After profiling confirms the comparison path matters and the runtime frequency is known, list common branches first without changing the set of handled values, fallback behavior, or branch bodies.
|
||||||
|
|
||||||
See sample: `order-case-branches-by-frequency.good.al`.
|
See sample: `order-case-branches-by-frequency.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Ordering branches alphabetically, by enum declaration order, or by "logical grouping" when the runtime distribution is heavily skewed. Every common record pays the cost of evaluating every uncommon branch first; on a posting routine processing thousands of rows the overhead is measurable.
|
Reordering branches based on assumed frequency without profiling, or changing an `else` arm or handled value while making the optimization. The good and bad forms must differ only in branch order.
|
||||||
|
|
||||||
See sample: `order-case-branches-by-frequency.bad.al`.
|
See sample: `order-case-branches-by-frequency.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -11,12 +11,12 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
A temporary table supports a full record API — filters, iteration, multi-field keys — but a pure key→value lookup pays for plumbing it does not use. Per the upstream guidance, "if a temporary table record is ONLY used as a lookup table, it is faster to use a dictionary which supports O(1) lookups instead of O(lg n) for temporary tables." The Dictionary type has no record machinery to traverse; the key hash answers the lookup directly.
|
An AL `Dictionary` directly models an unordered unique key-to-value collection. A temporary table models records and supports keys, filters, validation, and ordered iteration in Business Central Server memory. For a pure lookup map, the dictionary avoids repeatedly configuring and searching a temporary record and makes the intended access pattern explicit.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
When the use of a temp record is "set a key, see if the row exists, read a single value", switch to `Dictionary of [Key, Value]`. Use the temp-table form when the use genuinely needs filtering, iteration in a specific order, or a multi-field key. Compatibility with code that expects a `Record` parameter is a real reason to keep the temp table; performance alone, on a pure lookup, is not.
|
Use `Dictionary of [Key, Value]` when the operation is add-or-replace, contains-key, and get-value by one supported key type. Use a temporary table when the value is a record, or when the code needs filters, ordered iteration, multiple fields, multiple keys, or table behavior. Both structures consume service-tier memory and still need volume analysis.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
A temp `Record` declared, populated row by row, then queried with `SetRange(KeyField, X); if Find('=') then Value := Rec.ValueField;`. The lookup hashes the key behind the scenes and does the same work a `Dictionary` would, plus the per-row record overhead. The pattern often appears because the author originally needed iteration and the iteration was later removed without revisiting the data structure.
|
A temporary record used only through `SetRange(KeyField, X); FindFirst()` to retrieve one scalar value, with no record semantics that justify the table. The opposite mistake is replacing a temporary table that needs filtering or ordered iteration with a dictionary.
|
||||||
|
|
|
||||||
|
|
@ -1,15 +1,29 @@
|
||||||
|
table 50243 "Perf Import Staging Entry"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "Entry No."; Integer) { }
|
||||||
|
field(2; "Batch ID"; Guid) { }
|
||||||
|
field(3; Processed; Boolean) { }
|
||||||
|
}
|
||||||
|
|
||||||
|
keys
|
||||||
|
{
|
||||||
|
key(PK; "Entry No.") { Clustered = true; }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
codeunit 50243 "Perf Sample ModifyAll Bad"
|
codeunit 50243 "Perf Sample ModifyAll Bad"
|
||||||
{
|
{
|
||||||
procedure ApplyPriceUpdate(NewPrice: Decimal)
|
procedure MarkBatchProcessed(BatchId: Guid)
|
||||||
var
|
var
|
||||||
SalesLine: Record "Sales Line";
|
StagingEntry: Record "Perf Import Staging Entry";
|
||||||
begin
|
begin
|
||||||
SalesLine.SetRange(Type, SalesLine.Type::Item);
|
StagingEntry.SetRange("Batch ID", BatchId);
|
||||||
// N writes when one ModifyAll would do.
|
if StagingEntry.FindSet(true) then
|
||||||
if SalesLine.FindSet() then
|
|
||||||
repeat
|
repeat
|
||||||
SalesLine.Validate("Unit Price", NewPrice);
|
StagingEntry.Processed := true;
|
||||||
SalesLine.Modify(true);
|
StagingEntry.Modify(false);
|
||||||
until SalesLine.Next() = 0;
|
until StagingEntry.Next() = 0;
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,20 +1,26 @@
|
||||||
|
table 50242 "Perf Import Staging Entry"
|
||||||
|
{
|
||||||
|
fields
|
||||||
|
{
|
||||||
|
field(1; "Entry No."; Integer) { }
|
||||||
|
field(2; "Batch ID"; Guid) { }
|
||||||
|
field(3; Processed; Boolean) { }
|
||||||
|
}
|
||||||
|
|
||||||
|
keys
|
||||||
|
{
|
||||||
|
key(PK; "Entry No.") { Clustered = true; }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
codeunit 50242 "Perf Sample ModifyAll Good"
|
codeunit 50242 "Perf Sample ModifyAll Good"
|
||||||
{
|
{
|
||||||
procedure ApplyPriceUpdate(NewPrice: Decimal)
|
procedure MarkBatchProcessed(BatchId: Guid)
|
||||||
var
|
var
|
||||||
SalesLine: Record "Sales Line";
|
StagingEntry: Record "Perf Import Staging Entry";
|
||||||
begin
|
begin
|
||||||
SalesLine.SetRange(Type, SalesLine.Type::Item);
|
StagingEntry.SetRange("Batch ID", BatchId);
|
||||||
SalesLine.ModifyAll("Unit Price", NewPrice);
|
// Processed has no OnValidate logic, and the equivalent loop uses Modify(false).
|
||||||
end;
|
StagingEntry.ModifyAll(Processed, true, false);
|
||||||
|
|
||||||
procedure ApplyTolerance(DocumentNo: Code[20]; ToleranceAmount: Decimal)
|
|
||||||
var
|
|
||||||
CustLedgerEntry: Record "Cust. Ledger Entry";
|
|
||||||
begin
|
|
||||||
CustLedgerEntry.SetRange("Document No.", DocumentNo);
|
|
||||||
CustLedgerEntry.SetRange(Open, true);
|
|
||||||
CustLedgerEntry.ModifyAll("Accepted Payment Tolerance", ToleranceAmount);
|
|
||||||
CustLedgerEntry.ModifyAll("Accepted Pmt. Disc. Tolerance", false);
|
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -7,20 +7,20 @@ countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# Use ModifyAll / DeleteAll instead of per-row Modify / Delete in a loop
|
# Use ModifyAll only for equivalent bulk assignments
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
`ModifyAll` and `DeleteAll` are the bulk APIs. Per the upstream guidance, they "execute as single SQL statements" when the table supports it — one round-trip updates or deletes every row in the filtered set. The anti-pattern is the loop equivalent: `FindSet` followed by per-row `Modify`/`Delete`, where the runtime issues one write per row. On a production-scale table the difference is the difference between a single statement and N statements.
|
`ModifyAll` assigns one value to one field across the filtered set. It does not run the field's `OnValidate` trigger. Its optional `RunTrigger` parameter controls the table `OnModify` trigger, not field validation. Replacing a loop is therefore correct only when direct assignment is semantically equivalent for every row.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
When the loop body does nothing more than assign a constant value (or a value computed once) to one or more fields, replace the loop with `ModifyAll("Field 1", Value1)` — and chain additional `ModifyAll` calls for additional fields. The same shape applies to `DeleteAll`. Be aware that the bulk APIs can regress to row-by-row execution for tables with certain trigger or media-field configurations (see `triggers-and-media-field-regress-modifyall.md`); when that regression applies, multiple `ModifyAll` calls become more expensive than one manual loop, so the choice is conditional, not absolute.
|
Use `ModifyAll` when the loop directly assigns the same value, does not call `Validate`, needs no per-row calculation, and does not depend on `OnModify` unless the equivalent `RunTrigger` value is supplied. Check whether table-extension triggers, event subscribers, global triggers, or media fields force row-by-row fallback (see `triggers-and-media-field-regress-modifyall.md`).
|
||||||
|
|
||||||
See sample: `prefer-modifyall-over-per-row-modify.good.al`.
|
See sample: `prefer-modifyall-over-per-row-modify.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
`if SalesLine.FindSet() then repeat SalesLine.Validate("Unit Price", NewPrice); SalesLine.Modify(true); until SalesLine.Next() = 0;` — N writes when one would do. The pattern is easy to introduce when the loop initially does per-row computation and is later simplified to assign a constant; the loop scaffolding survives the simplification.
|
A loop that only assigns a constant and calls `Modify(false)` on a field with no validation side effects. Conversely, replacing `Validate(Field, Value); Modify(true)` with `ModifyAll(Field, Value)` is also an anti-pattern because it silently drops field validation and may drop table-trigger behavior.
|
||||||
|
|
||||||
See sample: `prefer-modifyall-over-per-row-modify.bad.al`.
|
See sample: `prefer-modifyall-over-per-row-modify.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -1,13 +1,13 @@
|
||||||
codeunit 50233 "Perf Sample ReadIso Bad"
|
codeunit 50233 "Perf Sample ReadIso Bad"
|
||||||
{
|
{
|
||||||
procedure GetOrCreate(var AgentStatus: Record "Agent Status")
|
procedure IsCustomerBlocked(CustomerNo: Code[20]): Boolean
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
begin
|
begin
|
||||||
// LockTable poisons every subsequent read of Agent Status in the
|
Customer.LockTable();
|
||||||
// surrounding transaction with UPDLOCK — even for callers that only read.
|
if not Customer.Get(CustomerNo) then
|
||||||
AgentStatus.LockTable();
|
exit(false);
|
||||||
if not AgentStatus.Get() then begin
|
|
||||||
AgentStatus.Init();
|
exit(Customer.Blocked <> Customer.Blocked::" ");
|
||||||
AgentStatus.Insert();
|
|
||||||
end;
|
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,11 +1,13 @@
|
||||||
codeunit 50232 "Perf Sample ReadIso Good"
|
codeunit 50232 "Perf Sample ReadIso Good"
|
||||||
{
|
{
|
||||||
procedure GetOrCreate(var AgentStatus: Record "Agent Status")
|
procedure IsCustomerBlocked(CustomerNo: Code[20]): Boolean
|
||||||
|
var
|
||||||
|
Customer: Record Customer;
|
||||||
begin
|
begin
|
||||||
AgentStatus.ReadIsolation := IsolationLevel::ReadCommitted;
|
Customer.ReadIsolation := IsolationLevel::ReadCommitted;
|
||||||
if not AgentStatus.Get() then begin
|
if not Customer.Get(CustomerNo) then
|
||||||
AgentStatus.Init();
|
exit(false);
|
||||||
AgentStatus.Insert();
|
|
||||||
end;
|
exit(Customer.Blocked <> Customer.Blocked::" ");
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -11,16 +11,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
`LockTable` and `ReadIsolation` solve different problems with different blast radii. Per the upstream guidance, "`LockTable` ensures that all READS against that table will happen with UPDLOCK for the remainder of the transaction." `ReadIsolation` "only pertains to the current record instance, while `LockTable` affects the lockstate of the entire transaction." `ReadIsolation` is also more expressive: it can heighten or lower the isolation level inside an already-established transaction. Reaching for `LockTable` when only a single read needs guarding therefore poisons every later read on that table — including reads in other code paths that share the transaction.
|
Without read scale-out, `LockTable` causes subsequent reads of that table in the transaction to use `UPDLOCK`. With read scale-out, those reads use `REPEATABLEREAD` on the replica instead. `ReadIsolation` selects an isolation level for one record instance. A helper that only reads should not broaden locking for the table merely to request committed data.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
For a read-only operation, or a single read that needs a higher isolation level than the surrounding transaction, set `Rec.ReadIsolation := IsolationLevel::ReadCommitted;` (or the level the call requires) immediately before the read. The hint applies only to that record instance. Save `LockTable` for code that genuinely needs every subsequent read on the table to acquire an update lock (see `findset-true-applies-updlock-on-read.md` for the alternative narrower mechanism on iterated reads).
|
For a read-only operation that specifically requires committed data, set `Rec.ReadIsolation := IsolationLevel::ReadCommitted` immediately before the read. If the default isolation is sufficient, set neither property. `ReadCommitted` can still block behind writers and does not guarantee that repeated reads stay unchanged; use the isolation level required by the operation. Reserve update locks for read-before-write logic, not read-only helpers.
|
||||||
|
|
||||||
See sample: `prefer-readisolation-over-locktable-for-reads.good.al`.
|
See sample: `prefer-readisolation-over-locktable-for-reads.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
`Rec.LockTable();` at the top of a helper that only reads, perhaps to "make sure the read is consistent". Every subsequent read on that table for the rest of the transaction acquires `UPDLOCK`, including reads from unrelated code paths fused into the same transaction. The contention surfaces in unrelated user sessions, not in the helper that introduced it.
|
`Rec.LockTable();` at the top of a helper that only reads, perhaps to "make sure the read is consistent". It takes stronger isolation than the helper needs and changes later reads of that table in the surrounding transaction or read-scale-out session.
|
||||||
|
|
||||||
See sample: `prefer-readisolation-over-locktable-for-reads.bad.al`.
|
See sample: `prefer-readisolation-over-locktable-for-reads.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -1,22 +0,0 @@
|
||||||
---
|
|
||||||
bc-version: [all]
|
|
||||||
domain: performance
|
|
||||||
keywords: [table-size, hot-table, ledger-entry, item, customer, sales-line, scale]
|
|
||||||
technologies: [al]
|
|
||||||
countries: [w1]
|
|
||||||
application-area: [all]
|
|
||||||
---
|
|
||||||
|
|
||||||
# Production-scale tables warrant concrete performance analysis
|
|
||||||
|
|
||||||
## Description
|
|
||||||
|
|
||||||
Some Business Central tables routinely reach sizes where access patterns matter much more than they do on a generic table. The upstream review guidance lists ten of them with P95 row counts: Item (~800k), Customer (~800k), Item Ledger Entry (~10M), Value Entry (~10M), G/L Entry (~10M), VAT Entry (~10M), Customer Ledger Entry (~10M), Vendor Ledger Entry (~10M), Sales Invoice Header (~300k), and Sales Invoice Line (~3M). These figures are not platform constants — they are the volumes a reviewer should assume when judging a change.
|
|
||||||
|
|
||||||
## Best Practice
|
|
||||||
|
|
||||||
For any code change that touches one of these tables, do not approve the pattern on intuition. Walk through the SQL the change implies (one query? one per row? one per chunk?), the memory it allocates (a `List` per row?), and the CPU work per row, against the row counts above. Smaller tables can tolerate a sub-optimal access pattern; these cannot. The rest of this domain — `apply-filters-before-iterating.md`, `use-setloadfields-for-partial-records.md`, `avoid-calcfields-in-loops.md`, `pair-findset-with-next-loop.md`, `avoid-get-inside-loop-on-persistent-tables.md` — exists primarily so that code touching these tables stays on the safe side of each rule.
|
|
||||||
|
|
||||||
## Anti Pattern
|
|
||||||
|
|
||||||
Generalizing from a unit test or a development tenant. A `FindSet` loop with a per-row `CalcFields` may execute in milliseconds against a few thousand rows on a developer's machine and become a multi-minute table scan against ten million Value Entry rows in production. Reasoning about performance from the dev-tenant timing instead of the production volume is the single most common way a regression ships.
|
|
||||||
|
|
@ -7,16 +7,16 @@ countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# Singleton setup tables hold one row; access-pattern optimization is wasted
|
# Enforced singleton setup tables need no access optimization
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
Business Central setup tables — `Sales & Receivables Setup`, `General Ledger Setup`, `FA Setup`, `Purchases & Payables Setup`, and the broader pattern of any `*Setup` table — hold at most one record per company. Per the upstream guidance, "any access pattern is fine, no `SetLoadFields` needed" on these tables. The same applies to other small bounded tables (enum mappings, permission objects, Role IDs) and system metadata tables (`TableMetadata`, `Field`, `AllObjWithCaption`) where iteration is safe.
|
An access-pattern exemption is valid only for a table whose schema and write paths enforce at most one row for the relevant scope. A conventional blank primary key, a parameterless `Get()`, or a table name ending in `Setup` does not enforce that invariant; another primary-key value can still create another row unless insertion logic prevents it.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Skip access-pattern optimization on singleton-setup-style tables. `SalesReceivablesSetup.Get()` does not need `SetLoadFields` (see `use-setloadfields-for-partial-records.md`); a `repeat ... until` over a permission-object table does not need bulk operations. Spend the review attention on the production-scale tables instead (see `production-scale-tables-warrant-extra-analysis.md`).
|
Exempt a setup read only after confirming that noncanonical keys are rejected and every supported creation path preserves the singleton. Otherwise apply ordinary access-pattern analysis, even when existing application code normally uses one blank-key record.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Mechanically applying the rules in this domain to every `Record` variable in the codebase. Flagging "missing `SetLoadFields`" on `GeneralLedgerSetup` or "use `IsEmpty` instead of `FindSet`" on a setup table adds noise without payoff — the optimization saves nothing measurable on a one-row table — and trains readers to ignore the review channel.
|
Treating every `*Setup` table or parameterless `Get()` as proof of bounded cardinality without checking the primary key and insertion logic.
|
||||||
|
|
|
||||||
|
|
@ -7,16 +7,16 @@ countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# Temporary tables are in-memory; access-pattern rules do not apply
|
# Temporary tables avoid SQL I/O, not in-memory work
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
A record declared `Temporary` (or a page with `SourceTableTemporary = true`) lives entirely in memory; reads and writes never reach SQL. Per the upstream guidance, "any access pattern (FindSet, FindFirst, Get, loops) on temp tables is acceptable — they are in-memory and fast." The rules in the rest of this domain — partial loading, bulk operations, N+1 detection, `IsEmpty` over `Count` — exist to avoid database round-trips that a temporary table does not perform.
|
A temporary table stores its rows in Business Central Server memory instead of a physical SQL table. Its reads and writes therefore do not incur SQL round-trips, locking, or SIFT maintenance. They still allocate memory and execute record filtering, key lookup, sorting, insertion, and iteration in the service tier; those costs grow with the temporary dataset and access pattern.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
Recognize the `Temporary` property (on a record variable, table declaration, or page's `SourceTableTemporary`) and exempt the code from access-pattern flags. The `SetLoadFields`/`FindSet` discipline that matters for `Customer` does not matter for a temporary `Customer` variable used as a working set. The interesting performance question on a temp table is volume in memory, not query plan.
|
Do not apply SQL-specific findings such as missing `SetLoadFields`, lock contention, or N+1 database round-trips to a temporary record. Still assess memory volume and repeated scans or lookups. For a pure key-to-value collection, consider an AL `Dictionary`; keep a temporary table when record fields, keys, filtering, or ordered iteration are required.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Flagging a temporary table's `FindFirst` inside a loop, or a temporary table without `SetLoadFields`, as a performance issue. The recommendation produces no measurable gain and obscures genuine issues elsewhere in the same review.
|
Claiming that every temporary-table access pattern is free because no SQL is involved. A nested scan over a large in-memory buffer can still dominate service-tier CPU, while adding `SetLoadFields` to that buffer addresses a database cost that does not exist.
|
||||||
|
|
|
||||||
|
|
@ -1,19 +1,29 @@
|
||||||
codeunit 50100 "Stale Quote Cleanup"
|
table 50100 "Perf Import Buffer"
|
||||||
{
|
{
|
||||||
procedure ClearExpiredQuotes(CutoffDate: Date)
|
fields
|
||||||
var
|
{
|
||||||
SalesHeader: Record "Sales Header";
|
field(1; "Entry No."; Integer) { }
|
||||||
begin
|
field(2; "Batch ID"; Guid) { }
|
||||||
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Quote);
|
field(3; Payload; Blob) { }
|
||||||
SalesHeader.SetFilter("Document Date", '<%1', CutoffDate);
|
}
|
||||||
SalesHeader.SetRange(Status, SalesHeader.Status::Open);
|
|
||||||
|
|
||||||
// One SQL DELETE per row. On a 10k-row cleanup, minutes instead of
|
keys
|
||||||
// under a second - and the OnDelete trigger has no logic this call
|
{
|
||||||
// needs to run.
|
key(PK; "Entry No.") { Clustered = true; }
|
||||||
if SalesHeader.FindSet() then
|
key(ByBatch; "Batch ID") { }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50100 "Perf Import Buffer Cleanup"
|
||||||
|
{
|
||||||
|
procedure ClearBatch(BatchId: Guid)
|
||||||
|
var
|
||||||
|
ImportBuffer: Record "Perf Import Buffer";
|
||||||
|
begin
|
||||||
|
ImportBuffer.SetRange("Batch ID", BatchId);
|
||||||
|
if ImportBuffer.FindSet() then
|
||||||
repeat
|
repeat
|
||||||
SalesHeader.Delete();
|
ImportBuffer.Delete(false);
|
||||||
until SalesHeader.Next() = 0;
|
until ImportBuffer.Next() = 0;
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,17 +1,28 @@
|
||||||
codeunit 50100 "Stale Quote Cleanup"
|
table 50100 "Perf Import Buffer"
|
||||||
{
|
{
|
||||||
procedure ClearExpiredQuotes(CutoffDate: Date)
|
fields
|
||||||
var
|
{
|
||||||
SalesHeader: Record "Sales Header";
|
field(1; "Entry No."; Integer) { }
|
||||||
begin
|
field(2; "Batch ID"; Guid) { }
|
||||||
// OnDelete on Sales Header carries no logic this call depends on:
|
field(3; Payload; Blob) { }
|
||||||
// expired quotes have no ledger entries, shipments, or downstream state.
|
}
|
||||||
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Quote);
|
|
||||||
SalesHeader.SetFilter("Document Date", '<%1', CutoffDate);
|
|
||||||
SalesHeader.SetRange(Status, SalesHeader.Status::Open);
|
|
||||||
|
|
||||||
// Single SQL DELETE. Orders of magnitude faster than FindSet + Delete
|
keys
|
||||||
// once the filtered set exceeds a handful of rows.
|
{
|
||||||
SalesHeader.DeleteAll();
|
key(PK; "Entry No.") { Clustered = true; }
|
||||||
|
key(ByBatch; "Batch ID") { }
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
codeunit 50100 "Perf Import Buffer Cleanup"
|
||||||
|
{
|
||||||
|
procedure ClearBatch(BatchId: Guid)
|
||||||
|
var
|
||||||
|
ImportBuffer: Record "Perf Import Buffer";
|
||||||
|
begin
|
||||||
|
ImportBuffer.SetRange("Batch ID", BatchId);
|
||||||
|
// This staging table has no base delete trigger. Installed extensions and
|
||||||
|
// subscribers must also be checked before assuming the set-based fast path.
|
||||||
|
ImportBuffer.DeleteAll(false);
|
||||||
end;
|
end;
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -13,16 +13,16 @@ application-area: [all]
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
`DeleteAll` translates to a single SQL `DELETE` with the record variable's current filters applied as the WHERE clause. A loop of `FindSet` + `Delete` instead issues one SQL statement per row. On any dataset larger than a handful of records, the gap is an order of magnitude or more. The tradeoff is that `DeleteAll` bypasses the `OnDelete` table trigger, so the decision hinges on whether that trigger's logic is required for this specific deletion.
|
`DeleteAll(false)` is eligible for a set-based SQL delete with the record variable's filters applied. It is not guaranteed to stay one statement. The base table `OnDelete` trigger is skipped, but table-extension `OnBeforeDelete` and `OnAfterDelete` triggers still run. Extension event subscribers, global delete triggers, and media fields can also require row processing. `DeleteAll(true)` runs the base table `OnDelete` trigger as well and has no performance advantage over `Delete(true)` in a loop.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
After narrowing the record set with `SetRange`/`SetFilter`, use `DeleteAll` whenever the `OnDelete` trigger has no logic that this call depends on — typically the case for housekeeping routines, staging-table cleanup, and deletions already validated upstream. When the trigger IS required, either keep the explicit loop-plus-`Delete` pattern and comment why, or pre-run the trigger logic against a temporary buffer and then `DeleteAll` the primary table.
|
Use filtered `DeleteAll(false)` for purpose-built staging or cleanup tables only after verifying that base-table `OnDelete` logic is unnecessary and installed extensions, subscribers, global triggers, and media fields do not add required per-row behavior or regress the bulk path. If deletion requires per-row business logic, keep an explicit triggered operation instead of simulating trigger execution separately.
|
||||||
|
|
||||||
See sample: `use-deleteall-for-filtered-bulk-deletion.good.al`.
|
See sample: `use-deleteall-for-filtered-bulk-deletion.good.al`.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
Iterating with `FindSet` + `Delete` to clear a filtered set of records that carry no meaningful `OnDelete` logic. Every row pays a full AL round-trip; on a ten-thousand-row cleanup the loop can take minutes where `DeleteAll` takes under a second.
|
Iterating with `FindSet` + `Delete(false)` to clear a filtered staging batch that has no delete logic. The reverse mistake is assuming `DeleteAll` is always one SQL statement without checking table extensions and subscribers.
|
||||||
|
|
||||||
See sample: `use-deleteall-for-filtered-bulk-deletion.bad.al`.
|
See sample: `use-deleteall-for-filtered-bulk-deletion.bad.al`.
|
||||||
|
|
|
||||||
|
|
@ -7,16 +7,16 @@ countries: [w1]
|
||||||
application-area: [all]
|
application-area: [all]
|
||||||
---
|
---
|
||||||
|
|
||||||
# Use TextBuilder for many string concatenations, especially inside loops
|
# Use AL TextBuilder for repeated text mutation
|
||||||
|
|
||||||
## Description
|
## Description
|
||||||
|
|
||||||
AL `Text` is immutable: each `Result += Piece;` allocates a new buffer and copies the previous content into it. Inside a loop the work is quadratic in the number of pieces. `TextBuilder` is the AL primitive designed for the pattern — per the upstream guidance, "Use `TextBuilder` when concatenating many strings together (for example inside loops)." Its `Append` mutates a growable internal buffer; `ToText()` materializes the final string once at the end.
|
AL `TextBuilder` is a reference type intended for modifying text without creating a new `Text` value for each change. Microsoft documents it as the performance-oriented AL primitive for concatenating many strings, including loop-built output. `Append` and `AppendLine` build the value, and `ToText` returns the completed text.
|
||||||
|
|
||||||
## Best Practice
|
## Best Practice
|
||||||
|
|
||||||
When a procedure assembles a string from many fragments — joining row data into a CSV, accumulating a log buffer, formatting a multi-line message inside a loop — declare a `TextBuilder` local, call `Append` per fragment, and call `ToText()` after the loop. For a fixed number of small fragments, `StrSubstNo` remains the right tool; the rule targets the loop case.
|
When a loop repeatedly appends fragments to one result, use a `TextBuilder` local and convert once after the loop. Keep ordinary `Text` expressions for a fixed, small number of fragments; this rule is about repeated mutation, not every concatenation.
|
||||||
|
|
||||||
## Anti Pattern
|
## Anti Pattern
|
||||||
|
|
||||||
`if Customer.FindSet() then repeat Csv += Customer."No." + ',' + Customer.Name + '\n'; until Customer.Next() = 0;` — every iteration reallocates and copies the entire string built so far. On a few hundred customers the cost is invisible; on the production-scale table list (`production-scale-tables-warrant-extra-analysis.md`) it dominates the loop.
|
Building an unbounded export or message with `Result += Fragment` on every iteration when AL's `TextBuilder` directly represents the operation.
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue