mirror of
https://github.com/microsoft/BCQuality.git
synced 2026-10-05 14:46:55 +01:00
Merge branch 'main' of https://github.com/demiliani/BCQuality into jobqueue
# Conflicts: # microsoft/knowledge/performance/job-queue-category-code-serializes-conflicting-jobs.bad.al # microsoft/knowledge/performance/job-queue-category-code-serializes-conflicting-jobs.good.al # microsoft/knowledge/performance/job-queue-category-code-serializes-conflicting-jobs.md # microsoft/knowledge/performance/job-queue-external-effects-must-be-idempotent.bad.al # microsoft/knowledge/performance/job-queue-external-effects-must-be-idempotent.good.al # microsoft/knowledge/performance/job-queue-external-effects-must-be-idempotent.md # microsoft/knowledge/performance/job-queue-handlers-must-not-require-ui.bad.al # microsoft/knowledge/performance/job-queue-handlers-must-not-require-ui.good.al # microsoft/knowledge/performance/job-queue-handlers-must-not-require-ui.md # microsoft/knowledge/performance/job-queue-handlers-must-propagate-failures.bad.al # microsoft/knowledge/performance/job-queue-handlers-must-propagate-failures.good.al # microsoft/knowledge/performance/job-queue-handlers-must-propagate-failures.md # microsoft/knowledge/performance/job-queue-on-hold-does-not-stop-running-work.bad.al # microsoft/knowledge/performance/job-queue-on-hold-does-not-stop-running-work.good.al # microsoft/knowledge/performance/job-queue-on-hold-does-not-stop-running-work.md # microsoft/knowledge/performance/store-scheduled-task-id-to-avoid-duplicate-tasks.bad.al # microsoft/knowledge/performance/store-scheduled-task-id-to-avoid-duplicate-tasks.good.al # microsoft/knowledge/performance/store-scheduled-task-id-to-avoid-duplicate-tasks.md
This commit is contained in:
commit
bb3398610e
228 changed files with 3304 additions and 432 deletions
|
|
@ -5,8 +5,8 @@ codeunit 50493 "Perf Record Clone Bad"
|
|||
Customer: Record Customer;
|
||||
CustomerCopy: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
if Customer.FindSet(true) then
|
||||
repeat
|
||||
CustomerCopy.Copy(Customer);
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ codeunit 50492 "Perf Record Clone Good"
|
|||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
if Customer.FindSet(true) then
|
||||
repeat
|
||||
Customer.Validate(
|
||||
|
|
|
|||
|
|
@ -3,12 +3,24 @@ codeunit 50129 "Perf Sample CommitInLoop Bad"
|
|||
procedure NormalizeCustomerNames()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
LastCustomerNo: Code[20];
|
||||
ProcessedCount: Integer;
|
||||
begin
|
||||
Customer.SetFilter("No.", '>%1', LastCustomerNo);
|
||||
if Customer.FindSet(true) then
|
||||
repeat
|
||||
Customer.Name := UpperCase(Customer.Name);
|
||||
Customer.Modify();
|
||||
Commit();
|
||||
|
||||
// LastCustomerNo exists only in memory, so a retry cannot exclude
|
||||
// work that was already committed.
|
||||
LastCustomerNo := Customer."No.";
|
||||
ProcessedCount += 1;
|
||||
|
||||
// This still opened a FindSet over the complete remaining tail;
|
||||
// periodic commits do not turn retrieval into bounded TOP X.
|
||||
if ProcessedCount mod 500 = 0 then
|
||||
Commit();
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -16,11 +16,22 @@ codeunit 50128 "Perf Sample CommitInLoop Good"
|
|||
{
|
||||
procedure NormalizeCustomerNames()
|
||||
var
|
||||
NormalizeState: Record "Perf Normalize State";
|
||||
LastCustomerNo: Code[20];
|
||||
begin
|
||||
// The outer loop owns checkpoints; the per-row loop contains no Commit.
|
||||
while NormalizeNextChunk(LastCustomerNo) do
|
||||
if not NormalizeState.Get('CUSTOMER') then begin
|
||||
NormalizeState.Init();
|
||||
NormalizeState.Code := 'CUSTOMER';
|
||||
NormalizeState.Insert();
|
||||
end;
|
||||
LastCustomerNo := NormalizeState."Last Customer No.";
|
||||
|
||||
while NormalizeNextChunk(LastCustomerNo) do begin
|
||||
// Persist progress in the same transaction as the completed chunk.
|
||||
NormalizeState."Last Customer No." := LastCustomerNo;
|
||||
NormalizeState.Modify();
|
||||
Commit();
|
||||
end;
|
||||
end;
|
||||
|
||||
local procedure NormalizeNextChunk(var LastCustomerNo: Code[20]): Boolean
|
||||
|
|
@ -58,3 +69,17 @@ codeunit 50128 "Perf Sample CommitInLoop Good"
|
|||
exit(true);
|
||||
end;
|
||||
}
|
||||
|
||||
table 50128 "Perf Normalize State"
|
||||
{
|
||||
fields
|
||||
{
|
||||
field(1; Code; Code[10]) { }
|
||||
field(2; "Last Customer No."; Code[20]) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; Code) { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,16 +13,18 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
Commit ends the current write transaction. Calling it inside a per-row loop produces one transaction per iteration and loses the ability to roll back the whole operation atomically; it also interferes with the platform's ability to batch write operations. Most loops need no explicit Commit at all — AL auto-commits the enclosing code module on successful completion (see `understand-implicit-transaction-boundary.md`). When the batch is too large for one transaction, the fix is not a per-row Commit but bounded checkpoints that select an exact list of at most N keys and process only those rows.
|
||||
Commit ends the current write transaction. Calling it inside a per-row loop usually produces one transaction per iteration and loses the ability to roll back the whole operation atomically; it also interferes with batching. Most loops need no explicit Commit at all — AL auto-commits the enclosing code module on successful completion (see `understand-implicit-transaction-boundary.md`).
|
||||
|
||||
A durability checkpoint inside an outer batch loop can be valid only when the same transaction persists a progress marker or state that makes retries strictly exclude completed work, the checkpoint follows a complete business unit, and errors propagate instead of being swallowed. Restart safety and bounded retrieval are separate requirements: a persisted watermark can make retries safe, but an outer `FindSet` over the full remaining tail with periodic commits still retrieves the complete set because [`FindSet` is not implemented as `TOP X`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/optimize-sql-al-database-methods-and-performance-on-server#get-find-findset-and-next).
|
||||
|
||||
## Best Practice
|
||||
|
||||
If the batch is large enough that a single transaction is untenable, use an ordered primary-key watermark and retrieve a bounded next-N key list. `FindSet` is optimized for reading the complete filtered set and isn't implemented as `TOP X`, so calling it over the remaining tail and breaking after N rows does not bound retrieval. The sample uses a query capped by [`TopNumberOfRows`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/query/queryinstance-topnumberofrows-method) to fill a temporary key buffer, then takes update locks and modifies only those exact keys. It does not reconstruct an inclusive first-to-last range that concurrent inserts could expand. Commit after the bounded inner loop returns and persist its last selected key as the next watermark. Use a stable key and define how a later run handles records inserted at or below an already committed watermark. 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`.
|
||||
If the batch is large enough that a single transaction is untenable, use an ordered primary-key watermark and retrieve a bounded next-N key list. The sample uses a query capped by [`TopNumberOfRows`](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/query/queryinstance-topnumberofrows-method) to fill a temporary key buffer, then takes update locks and modifies only those exact keys. It does not reconstruct an inclusive first-to-last range that concurrent inserts could expand. Persist the last selected key in the same transaction as the completed chunk, then commit after the bounded helper returns. Use a stable key and define how a later run handles records inserted at or below an already committed watermark. Let errors escape so failed work is not recorded as complete. 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`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
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. A capped query that discovers only an upper key and then re-reads an inclusive key range is not exact batching either; concurrent inserts inside that range can enlarge the checkpoint.
|
||||
Placing Commit inside `repeat ... until Next() = 0` without persisted progress is almost always a mistake: retries re-enter already committed work, while the cost of starting a transaction on every row dominates the operation. A progress variable held only in memory is not restart-safe. A full-tail `FindSet` with a commit every N rows is not bounded retrieval, even if a persisted watermark makes it restart-safe. A capped query that discovers only an upper key and then re-reads an inclusive key range is not exact batching either; concurrent inserts inside that range can enlarge the checkpoint.
|
||||
|
||||
See sample: `avoid-commit-inside-loops.bad.al`.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,24 @@
|
|||
page 50100 "CurrPage Update OAGR Bad"
|
||||
{
|
||||
PageType = List;
|
||||
SourceTable = Customer;
|
||||
ApplicationArea = All;
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
repeater(Rows)
|
||||
{
|
||||
field("No."; Rec."No.") { }
|
||||
field(Name; Rec.Name) { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
trigger OnAfterGetRecord()
|
||||
begin
|
||||
// Update from OnAfterGetRecord re-enters the trigger on every row.
|
||||
CurrPage.Update(false);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,26 @@
|
|||
page 50100 "CurrPage Update OAGR Good"
|
||||
{
|
||||
PageType = List;
|
||||
SourceTable = Customer;
|
||||
ApplicationArea = All;
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
repeater(Rows)
|
||||
{
|
||||
field("No."; Rec."No.") { }
|
||||
field(Warning; WarningText) { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var
|
||||
WarningText: Text[50];
|
||||
|
||||
trigger OnAfterGetRecord()
|
||||
begin
|
||||
WarningText := CopyStr(Rec.Name, 1, MaxStrLen(WarningText));
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [currpage-update, onaftergetrecord, list-page, scroll, refresh]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not call CurrPage.Update inside OnAfterGetRecord
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`OnAfterGetRecord` on a list already runs once per visible row on scroll and refresh. `CurrPage.Update` asks the page to reload, which fires those triggers again. The result is a refresh loop or a stutter on every row paint. Official developer performance guidance lists `CurrPage.Update()` in `OnAfterGetRecord` next to `Modify` as work that must not live there. Sibling of `do-not-modify-in-onaftergetrecord.md` (writes); this file is the client refresh half.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Put display-only results in page variables assigned in `OnAfterGetRecord` without calling `Update`. If the page must refresh after an action, call `CurrPage.Update(false)` from `OnAction` once, not per row.
|
||||
|
||||
See sample: `avoid-currpage-update-in-onaftergetrecord.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`trigger OnAfterGetRecord() begin ... CurrPage.Update(); end;` on a list. The signal is `CurrPage.Update` inside `OnAfterGetRecord` or `OnAfterGetCurrRecord` without an explicit user action.
|
||||
|
||||
See sample: `avoid-currpage-update-in-onaftergetrecord.bad.al`.
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
codeunit 50100 "Batch NoSeries Insert Bad"
|
||||
{
|
||||
procedure InsertDraftOrders(var Customer: Record Customer)
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
SalesSetup: Record "Sales & Receivables Setup";
|
||||
NoSeries: Codeunit "No. Series";
|
||||
begin
|
||||
SalesSetup.Get();
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
SalesHeader.Init();
|
||||
SalesHeader."Document Type" := SalesHeader."Document Type"::Order;
|
||||
// Per-row GetNextNo locks the number-series line every insert.
|
||||
SalesHeader."No." := NoSeries.GetNextNo(SalesSetup."Order Nos.", WorkDate());
|
||||
SalesHeader."Sell-to Customer No." := Customer."No.";
|
||||
SalesHeader.Insert(true);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
codeunit 50100 "Batch NoSeries Insert Good"
|
||||
{
|
||||
procedure InsertDraftOrders(var Customer: Record Customer)
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
SalesSetup: Record "Sales & Receivables Setup";
|
||||
NoSeriesBatch: Codeunit "No. Series - Batch";
|
||||
begin
|
||||
SalesSetup.Get();
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
SalesHeader.Init();
|
||||
SalesHeader."Document Type" := SalesHeader."Document Type"::Order;
|
||||
SalesHeader."No." := NoSeriesBatch.GetNextNo(SalesSetup."Order Nos.", WorkDate());
|
||||
SalesHeader."Sell-to Customer No." := Customer."No.";
|
||||
SalesHeader.Insert(true);
|
||||
until Customer.Next() = 0;
|
||||
NoSeriesBatch.SaveState();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [22..]
|
||||
domain: performance
|
||||
keywords: [no-series, getnextno, no-series-batch, savestate, numbersequence, lock]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Batch number-series calls instead of GetNextNo per insert
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`Codeunit "No. Series".GetNextNo` on a **gapless (Normal)** series updates and locks the number-series line on every call. A tight `Insert` loop that asks for a number per row serializes every concurrent writer on that series — the classic SaaS posting bottleneck. Training data still copies the per-row C/AL `NoSeriesManagement` shape. Series configured with **Allow Gaps** instead obtain numbers through `NumberSequence` and do not hold the series-line lock between calls, so they are not affected by this pattern. Codeunit `"No. Series - Batch"` issues gapless numbers in memory and writes the series line once via `SaveState`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Inside a multi-row insert, call `"No. Series - Batch".GetNextNo` per row and `SaveState` once after the loop when the series must remain gapless. Use `NumberSequence.Next` when holes are allowed. Do not replace a single `OnInsert` `GetNextNo` for one master record; that path is not the hotspot.
|
||||
|
||||
See sample: `batch-number-series-instead-of-getnextno-per-row.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`NoSeries.GetNextNo(...)` inside `repeat ... Insert ... until Next() = 0` where the series is **gapless** (Allow Gaps = false). Each iteration takes the series-line lock. The signal is `"No. Series"` (not `"No. Series - Batch"`) in a loop that inserts more than one row; do not flag the same pattern when the series has Allow Gaps enabled, as the `NumberSequence` path already avoids the lock.
|
||||
|
||||
See sample: `batch-number-series-instead-of-getnextno-per-row.bad.al`.
|
||||
|
|
@ -0,0 +1,31 @@
|
|||
codeunit 50541 "Perf Sample NoShortCircuit Bad"
|
||||
{
|
||||
procedure ExceedsThreshold(var Thresholds: array[10] of Decimal; Index: Integer; Amount: Decimal): Boolean
|
||||
begin
|
||||
// Thresholds[Index] is evaluated even when Index is 0, so the leading range
|
||||
// check does not prevent the subscript from being read out of range.
|
||||
exit((Index >= 1) and (Index <= ArrayLen(Thresholds)) and (Amount > Thresholds[Index]));
|
||||
end;
|
||||
|
||||
procedure IsBlockedCustomer(CustomerNo: Code[20]): Boolean
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
// The Get runs even for an empty CustomerNo, and Blocked is read even when the
|
||||
// Get failed, so the result is taken from a record that was never loaded.
|
||||
exit((CustomerNo <> '') and Customer.Get(CustomerNo) and (Customer.Blocked <> Customer.Blocked::" "));
|
||||
end;
|
||||
|
||||
procedure IsEligibleForFreeShipping(SalesHeader: Record "Sales Header"): Boolean
|
||||
begin
|
||||
// HasActiveLoyaltyBenefit runs even when the amount alone already qualifies,
|
||||
// paying for the costly check on every evaluation instead of only the path
|
||||
// where it can still change the outcome.
|
||||
exit((SalesHeader."Amount Including VAT" >= 1000) or HasActiveLoyaltyBenefit(SalesHeader."Sell-to Customer No."));
|
||||
end;
|
||||
|
||||
local procedure HasActiveLoyaltyBenefit(CustomerNo: Code[20]): Boolean
|
||||
begin
|
||||
exit(CustomerNo <> '');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,41 @@
|
|||
codeunit 50540 "Perf Sample NoShortCircuit Good"
|
||||
{
|
||||
procedure ExceedsThreshold(var Thresholds: array[10] of Decimal; Index: Integer; Amount: Decimal): Boolean
|
||||
begin
|
||||
// 'and' is safe here: both operands are cheap and neither depends on the other.
|
||||
if (Index >= 1) and (Index <= ArrayLen(Thresholds)) then
|
||||
// The subscript lives in its own if, so it is never evaluated out of range.
|
||||
if Amount > Thresholds[Index] then
|
||||
exit(true);
|
||||
exit(false);
|
||||
end;
|
||||
|
||||
procedure IsBlockedCustomer(CustomerNo: Code[20]): Boolean
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
// The cheap test runs first, and the field is read only after Get succeeded.
|
||||
if CustomerNo = '' then
|
||||
exit(false);
|
||||
if not Customer.Get(CustomerNo) then
|
||||
exit(false);
|
||||
exit(Customer.Blocked <> Customer.Blocked::" ");
|
||||
end;
|
||||
|
||||
procedure IsEligibleForFreeShipping(SalesHeader: Record "Sales Header"): Boolean
|
||||
begin
|
||||
// 'or' is unsafe here: nesting would also be wrong, since it would drop the
|
||||
// case where the amount alone already qualifies. Exit as soon as the cheap
|
||||
// condition already decides the result; the costly lookup runs only on the
|
||||
// path where it can still change the outcome.
|
||||
if SalesHeader."Amount Including VAT" >= 1000 then
|
||||
exit(true);
|
||||
exit(HasActiveLoyaltyBenefit(SalesHeader."Sell-to Customer No."));
|
||||
end;
|
||||
|
||||
local procedure HasActiveLoyaltyBenefit(CustomerNo: Code[20]): Boolean
|
||||
begin
|
||||
// Stands in for a costly check — a webservice call or a large table scan.
|
||||
exit(CustomerNo <> '');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,36 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [short-circuit, lazy-evaluation, boolean-operators, nested-if, guard, and-operator, or-operator, xor-operator, early-exit]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# AL boolean operators do not short-circuit
|
||||
|
||||
## Description
|
||||
|
||||
AL gives no short-circuit (lazy) evaluation guarantee for `and`, `or`, and `xor`: every operand of a boolean expression is evaluated, even when the leftmost operand already determines the result. Neither the AL operators documentation nor the boolean operators documentation defines a lazy evaluation order, so code must not depend on one. Developers arriving from C#, JavaScript, or SQL routinely assume the left operand guards the right; in AL it does not. `xor` is not actually a short-circuit candidate in any language — its result depends on both operands regardless of their values, so there is nothing to skip — but AL still evaluates both operands unconditionally, so neither should carry a cost or a risk the developer assumed the other would guard against. For `and` and `or`, the right operand still runs even when the left already decides the result, so its cost is paid on every evaluation, and a check intended to protect an unsafe expression — an array subscript, a division, a field read that is only valid after a successful `Get` — does not protect it.
|
||||
|
||||
## Best Practice
|
||||
|
||||
For an `and`-shaped guard — a condition that must hold before the next operand is safe or worth evaluating — split into nested `if` statements: the guarding or cheapest condition in the outer `if`, the dependent or expensive one in the inner `if`. This preserves the result, since `if A then if B then Action` matches `if A and B then Action` exactly. Where there is no `else` branch, nesting is a pure win; where there is one, extract the conditions into a helper procedure that exits early instead.
|
||||
|
||||
For an `or`-shaped condition, do not nest: nesting `if A then if B then Action` drops the case where `A` is true and `B` is false, silently changing the result of `A or B`. Exit as soon as the cheap or safe operand already decides the outcome, and reach the other operand only on the path where it can still change the result — `if A then exit(true); exit(B);` for a boolean return, or `if A then Action else if B then Action;` when both branches share one action.
|
||||
|
||||
`xor` has no equivalent rewrite, because its result always depends on both operands; the only actionable guidance is to keep both operands of an `xor` cheap and free of side effects, since AL evaluates both unconditionally.
|
||||
|
||||
Where a chain of `and`-guards runs past about three conditions, stop nesting and use a `case` statement instead — see `case-true-of-for-long-condition-chains.md`. Keep `and` and `or` for operands that are independently safe and cheap — in-memory field comparisons, enum tests, bound checks — where combining them reads better and costs nothing.
|
||||
|
||||
See sample: `boolean-operators-do-not-short-circuit.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A single condition that joins a guard with an operand depending on that guard, or with an expensive operand, using `and` or `or`. The consequence is either wasted work on every evaluation — a database call or validation procedure invoked even when the outcome is already decided — or a runtime error or silently wrong result that the guard was written to prevent. Applying the `and` fix to an `or` condition is a distinct mistake: rewriting `A or B` as nested `if`s drops the `A`-true/`B`-false case instead of preserving it. Detection signals: an operand that indexes an array or list with a variable whose bounds are checked in a sibling operand; `Record.Get(...)` or a `Find`/`IsEmpty` call as one operand of `and` with a field read of the same record as another; an expensive or unsafe operand combined with `or` next to a condition that alone already makes the result true; a boolean-returning procedure call combined with a cheap field test. The pattern is common in code ported from a language that does short-circuit, and in conditions grown by appending a clause to an existing `if`.
|
||||
|
||||
See sample: `boolean-operators-do-not-short-circuit.bad.al`.
|
||||
|
||||
## See also
|
||||
|
||||
`case-true-of-for-long-condition-chains.md` covers what to do when nesting an `and`-guard chain would go more than about three levels deep. `microsoft/knowledge/performance/apply-guards-before-get.md` covers the related ordering rule for statements rather than operands.
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
codeunit 50543 "Perf Sample CaseChain Bad"
|
||||
{
|
||||
procedure IsShippableLine(SalesLine: Record "Sales Line"): Boolean
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
// Five levels of nesting to sequence five guards. The evaluation order is
|
||||
// carried by indentation alone and the body drifts steadily right.
|
||||
if SalesLine.Type = SalesLine.Type::Item then
|
||||
if SalesLine."No." <> '' then
|
||||
if SalesLine."Qty. to Ship" > 0 then
|
||||
if Item.Get(SalesLine."No.") then
|
||||
if not Item.Blocked then
|
||||
exit(true);
|
||||
exit(false);
|
||||
end;
|
||||
|
||||
procedure IsShippableLineCollapsed(SalesLine: Record "Sales Line"): Boolean
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
// The wrong escape from the ladder: flattening it into 'and' trades the
|
||||
// nesting for a defect, because every operand is still evaluated. Item
|
||||
// fields are read even when the Get failed. The parentheses are not
|
||||
// optional either — 'and' binds tighter than '=' and '<>' in AL.
|
||||
exit((SalesLine.Type = SalesLine.Type::Item) and (SalesLine."No." <> '') and
|
||||
(SalesLine."Qty. to Ship" > 0) and Item.Get(SalesLine."No.") and not Item.Blocked);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,48 @@
|
|||
codeunit 50542 "Perf Sample CaseChain Good"
|
||||
{
|
||||
procedure IsShippableLine(SalesLine: Record "Sales Line"): Boolean
|
||||
var
|
||||
Item: Record Item;
|
||||
begin
|
||||
// 'case false of' matches value sets in order and stops at the first match.
|
||||
// The first three checks are pure and order-independent, so they share one
|
||||
// value set. Get and Blocked are each their own value set, in order, because
|
||||
// the ordering the documentation guarantees is across value sets, not within
|
||||
// one — Item.Get must run, and succeed, before Blocked is read.
|
||||
case false of
|
||||
SalesLine.Type = SalesLine.Type::Item,
|
||||
SalesLine."No." <> '',
|
||||
SalesLine."Qty. to Ship" > 0:
|
||||
exit(false);
|
||||
Item.Get(SalesLine."No."):
|
||||
exit(false);
|
||||
not Item.Blocked:
|
||||
exit(false);
|
||||
end;
|
||||
exit(true);
|
||||
end;
|
||||
|
||||
procedure FindOpenDocumentType(CustomerNo: Code[20]): Text
|
||||
begin
|
||||
// 'case true of' stops at the first condition that holds, so the later
|
||||
// lookups never run once an earlier one matched.
|
||||
case true of
|
||||
HasOpenDocument(CustomerNo, "Sales Document Type"::Quote):
|
||||
exit('Quote');
|
||||
HasOpenDocument(CustomerNo, "Sales Document Type"::Order):
|
||||
exit('Order');
|
||||
HasOpenDocument(CustomerNo, "Sales Document Type"::Invoice):
|
||||
exit('Invoice');
|
||||
end;
|
||||
exit('None');
|
||||
end;
|
||||
|
||||
local procedure HasOpenDocument(CustomerNo: Code[20]; DocumentType: Enum "Sales Document Type"): Boolean
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
SalesHeader.SetRange("Document Type", DocumentType);
|
||||
SalesHeader.SetRange("Sell-to Customer No.", CustomerNo);
|
||||
exit(not SalesHeader.IsEmpty());
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [case-statement, case-true-of, nested-if, condition-chain, guard, lazy-evaluation, nesting-depth]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use case true of for long chains of dependent conditions
|
||||
|
||||
## Description
|
||||
|
||||
Because AL gives no short-circuit guarantee for `and` and `or`, a chain of conditions that must be evaluated in order has to be sequenced with nested `if` statements — and past three conditions the nesting itself becomes the problem: the body drifts right, the order of evaluation is carried by indentation alone, and any shared failure path is repeated at every level. AL's `case` statement is the flat alternative. Its value sets "must be an expression or a range", so `case true of` and `case false of` accept arbitrary boolean expressions, and the statement "is evaluated, and the first matching value set executes the associated statement" — evaluation stops at the first matching value set, which is exactly the laziness the boolean operators do not provide. That guarantee is stated for value sets, plural: it orders evaluation *across* separate value sets, and says nothing about the order of the individual expressions listed inside one comma-separated value set.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Sequence two or three dependent conditions with nested `if`. Beyond that, switch to `case`: use `case false of` for a chain of guards where every condition must hold, letting control fall past `end` when all of them pass; use `case true of` for first-match dispatch, where each later probe runs only if the earlier ones did not match. Comma-separate conditions into one value set only when every one of them is a pure, order-independent test with no side effect — a field comparison, an enum check, a bound test — so it makes no difference whether AL evaluates all of them or stops early; grouping these costs nothing and removes the repeated action. A condition that guards another, or that carries a side effect or a cost of its own — a `Get`, a `Find`, a procedure call — keeps its own value set, placed immediately after the value set it depends on, so the code relies only on the ordering the documentation actually states. A value set needs no parentheses around a comparison, unlike an operand of `and` or `or`: the AL operator hierarchy places `and` and `or` above the comparison operators, so parentheses are mandatory there and the chain fills up with them. This keeps every condition at one indentation level, makes evaluation order explicit rather than implied by nesting, and preserves the stop-at-first-match behaviour it relies on. It also aligns with the AL programming convention that more than two alternatives belong in a `case` statement rather than an `if-then-else`.
|
||||
|
||||
See sample: `case-true-of-for-long-condition-chains.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
An `if` ladder four or more levels deep whose only purpose is sequencing guards. Detection: a chain of nested `if` statements with no `else`, each condition guarding the one below it, terminating in a single action or `exit`; or the same `exit`/`error` duplicated at every level of such a nested chain, purely to escape it. The second, worse form is collapsing that ladder into one `and` chain to escape the nesting — that trades indentation for a real defect, because the operands are still all evaluated. A third, subtler form is over-applying the comma-grouping itself: putting a guard and the condition it protects — for example `Item.Get(...)` and a read of a field on that same record — into one comma-separated value set. That relies on an evaluation order within a single value set that the documentation does not state; keep them in separate value sets instead. Reach for `case` over nested `if` or a collapsed `and` chain, and keep order-dependent conditions in their own value sets within it.
|
||||
|
||||
See sample: `case-true-of-for-long-condition-chains.bad.al`.
|
||||
|
||||
## See also
|
||||
|
||||
`boolean-operators-do-not-short-circuit.md` covers the underlying evaluation rule that makes the sequencing necessary in the first place.
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
codeunit 50100 "ChangeCompany Loop Bad"
|
||||
{
|
||||
procedure NamesForCustomers(var Buffer: Record Customer)
|
||||
var
|
||||
Customer: Record Customer;
|
||||
Company: Record Company;
|
||||
begin
|
||||
Customer.SetLoadFields(Name);
|
||||
if Buffer.FindSet() then
|
||||
repeat
|
||||
if Company.FindSet() then
|
||||
repeat
|
||||
// ChangeCompany per customer per company resets caches every row.
|
||||
Customer.ChangeCompany(Company.Name);
|
||||
if Customer.Get(Buffer."No.") then
|
||||
Message(Customer.Name);
|
||||
until Company.Next() = 0;
|
||||
until Buffer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
codeunit 50100 "ChangeCompany Loop Good"
|
||||
{
|
||||
procedure NamesForCustomers(var Buffer: Record Customer)
|
||||
var
|
||||
Customer: Record Customer;
|
||||
Company: Record Company;
|
||||
begin
|
||||
Customer.SetLoadFields(Name);
|
||||
if Company.FindSet() then
|
||||
repeat
|
||||
Customer.ChangeCompany(Company.Name);
|
||||
if Buffer.FindSet() then
|
||||
repeat
|
||||
if Customer.Get(Buffer."No.") then
|
||||
Message(Customer.Name);
|
||||
until Buffer.Next() = 0;
|
||||
until Company.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [changecompany, loop, cache, multi-company, isolation]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not call ChangeCompany inside a per-row loop
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`ChangeCompany` retargets a record variable to another company's data and drops the in-memory caches bound to the previous company. Calling it once per row in a multi-company scan therefore pays a cache reset on every iteration, even when consecutive rows share a company. Agents treat `ChangeCompany` like a filter. It is an isolation switch.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Group work by company. Call `ChangeCompany` once per distinct company, then `FindSet`/`Get` that company's rows. If the record variable is reused afterward, call `ChangeCompany()` without a company name to redirect it back to the current company.
|
||||
|
||||
See sample: `changecompany-in-loop-drops-caches.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`repeat Rec.ChangeCompany(Buffer.Company); Rec.Get(Buffer."No."); until Buffer.Next() = 0` when `Buffer` is not ordered by company, or even when it is — if `ChangeCompany` still runs every row. The signal is `ChangeCompany` inside `repeat`/`while` keyed by a document line rather than by a company loop.
|
||||
|
||||
See sample: `changecompany-in-loop-drops-caches.bad.al`.
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
report 50100 "Cust List ReadOnly Bad"
|
||||
{
|
||||
UsageCategory = ReportsAndAnalysis;
|
||||
ApplicationArea = All;
|
||||
// Missing DataAccessIntent = ReadOnly; the scan hits the primary replica.
|
||||
|
||||
dataset
|
||||
{
|
||||
dataitem(Customer; Customer)
|
||||
{
|
||||
column(No; "No.") { }
|
||||
column(Name; Name) { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
report 50100 "Cust List ReadOnly Good"
|
||||
{
|
||||
UsageCategory = ReportsAndAnalysis;
|
||||
ApplicationArea = All;
|
||||
DataAccessIntent = ReadOnly;
|
||||
|
||||
dataset
|
||||
{
|
||||
dataitem(Customer; Customer)
|
||||
{
|
||||
column(No; "No.") { }
|
||||
column(Name; Name) { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: ["16.."]
|
||||
domain: performance
|
||||
keywords: [dataaccessintent, read-only, read-scale-out, report, api-page, query]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Set DataAccessIntent ReadOnly on analytical objects
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`DataAccessIntent` was introduced at runtime 5.0 (BC 16) and has no effect in earlier versions. Reports, API pages (`PageType = API` with `Editable = false`), and queries that only read can run against a read replica when `DataAccessIntent = ReadOnly`. For queries, replica routing only applies when the query is exposed via OData/API; running a query in AL code is unaffected. Without the property these objects hit the primary replica and compete with posting. Agents omit it because the default is read-write and the object "only reads" in AL. The replica routing is a metadata switch, not something the compiler infers from the absence of `Modify`.
|
||||
|
||||
## Best Practice
|
||||
|
||||
On report objects and `PageType = API` pages with `Editable = false` that never write, set `DataAccessIntent = ReadOnly`. For query objects, set it when the query is consumed via OData or an API endpoint. Keep the default on objects that insert, modify, or call a write codeunit from a processing-only report.
|
||||
|
||||
See sample: `dataaccessintent-readonly-on-analytical-objects.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A listing report or API query with no `DataAccessIntent` that scans G/L or sales lines. The object is read-only in practice and still loads the primary.
|
||||
|
||||
See sample: `dataaccessintent-readonly-on-analytical-objects.bad.al`.
|
||||
|
|
@ -0,0 +1,34 @@
|
|||
page 50100 "GuiAllowed OData Guard Bad"
|
||||
{
|
||||
PageType = List;
|
||||
SourceTable = Customer;
|
||||
ApplicationArea = All;
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
repeater(Rows)
|
||||
{
|
||||
field("No."; Rec."No.") { }
|
||||
field(Name; Rec.Name)
|
||||
{
|
||||
StyleExpr = NameStyle;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var
|
||||
NameStyle: Text;
|
||||
|
||||
trigger OnAfterGetRecord()
|
||||
begin
|
||||
// UI-only styling still runs for every OData / Edit-in-Excel row.
|
||||
Rec.CalcFields("Balance (LCY)");
|
||||
if Rec."Balance (LCY)" > 0 then
|
||||
NameStyle := 'Attention'
|
||||
else
|
||||
NameStyle := 'Standard';
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,35 @@
|
|||
page 50100 "GuiAllowed OData Guard Good"
|
||||
{
|
||||
PageType = List;
|
||||
SourceTable = Customer;
|
||||
ApplicationArea = All;
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
repeater(Rows)
|
||||
{
|
||||
field("No."; Rec."No.") { }
|
||||
field(Name; Rec.Name)
|
||||
{
|
||||
StyleExpr = NameStyle;
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var
|
||||
NameStyle: Text;
|
||||
|
||||
trigger OnAfterGetRecord()
|
||||
begin
|
||||
if not GuiAllowed then
|
||||
exit;
|
||||
Rec.CalcFields("Balance (LCY)");
|
||||
if Rec."Balance (LCY)" > 0 then
|
||||
NameStyle := 'Attention'
|
||||
else
|
||||
NameStyle := 'Standard';
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [guiallowed, clienttype, odata, edit-in-excel, page-trigger, factbox]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Guard page trigger work with GuiAllowed for OData and Excel
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Pages exposed as OData, including Edit in Excel, still run AL page triggers for every row returned. FactBox updates, defaulting, and extra `CalcFields` in `OnAfterGetRecord` therefore run on the web-service path where no UI exists. `GuiAllowed` is false for those sessions. Agents add page logic as if only the browser client will execute it.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Wrap UI-only work — FactBox refresh, notifications, defaulting that is not part of the web-service contract — in `if GuiAllowed then`. Keep the OData path to field values the API actually returns.
|
||||
|
||||
See sample: `guiallowed-guard-on-pages-used-as-odata.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Unconditional FactBox or calculation logic in `OnAfterGetRecord` / `OnAfterGetCurrRecord` on a page that is published as a web service or used with Edit in Excel. The signal is trigger work that calls `CurrPage` parts or extra queries without a `GuiAllowed` guard.
|
||||
|
||||
See sample: `guiallowed-guard-on-pages-used-as-odata.bad.al`.
|
||||
|
|
@ -0,0 +1,13 @@
|
|||
codeunit 50100 "HttpClient Holds Locks Bad"
|
||||
{
|
||||
procedure SyncCustomerLastName(var Customer: Record Customer)
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
Customer."Search Name" := Customer.Name;
|
||||
Customer.Modify(false);
|
||||
// Locks from Modify are held for the entire HTTP wait.
|
||||
Client.Get(StrSubstNo('https://example.local/sync/%1', Customer."No."), Response);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,62 @@
|
|||
codeunit 50100 "HttpClient Holds Locks Good"
|
||||
{
|
||||
procedure SyncCustomerLastName(var Customer: Record Customer)
|
||||
var
|
||||
CustomerSyncOutbox: Record "Customer Sync Outbox";
|
||||
begin
|
||||
Customer."Search Name" := Customer.Name;
|
||||
Customer.Modify(false);
|
||||
|
||||
// This work item commits or rolls back with the customer change.
|
||||
CustomerSyncOutbox."Customer No." := Customer."No.";
|
||||
CustomerSyncOutbox.Insert();
|
||||
end;
|
||||
}
|
||||
|
||||
table 50100 "Customer Sync Outbox"
|
||||
{
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(2; "Customer No."; Code[20]) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50101 "Customer Sync Outbox Worker"
|
||||
{
|
||||
// Configure this codeunit as a recurring job queue entry.
|
||||
TableNo = "Job Queue Entry";
|
||||
|
||||
trigger OnRun()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
CustomerSyncOutbox: Record "Customer Sync Outbox";
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
// Only committed work is visible here; a rolled-back change leaves no outbox row.
|
||||
if not CustomerSyncOutbox.FindFirst() then
|
||||
exit;
|
||||
|
||||
Customer.Get(CustomerSyncOutbox."Customer No.");
|
||||
Client.Get(StrSubstNo('https://example.local/sync/%1', Customer."No."), Response);
|
||||
if not Response.IsSuccessStatusCode() then
|
||||
Error('Customer sync failed with HTTP status %1.', Response.HttpStatusCode());
|
||||
|
||||
// Delete only after HTTP completes, so no write lock is held during the call.
|
||||
CustomerSyncOutbox.Delete();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [httpclient, write-transaction, lock, commit, outbound-http, session-block]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Do not call HttpClient inside an open write transaction
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
The first database write opens an AL write transaction that the runtime holds until the execution completes or `Commit()` runs — see `understand-implicit-transaction-boundary.md`. `HttpClient` blocks the session until the remote call returns. Any locks taken by earlier `Insert`/`Modify`/`Delete` therefore stay held for the HTTP wall-clock time, and interactive users see a spinner. This is not generic "don't block": it is the AL transaction model plus lock lifetime around outbound I/O.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Defer the HTTP call to a separate session. When the external operation must correspond to a committed database change, insert an outbox work item in the same transaction as that change and process committed outbox rows with a recurring job queue entry. The change and work item then commit or roll back together, and the worker performs HTTP before deleting the item so it holds no write lock during the call. The separate retry-safety requirement is covered by `job-queue-external-effects-must-be-idempotent.md`.
|
||||
|
||||
A directly created scheduled task is suitable only when its work is independent of the caller's commit. An immediately ready task can run concurrently with the caller, so it must not assume that the caller's writes are already committed. Do **not** use `Commit()` as a general remedy: it irrevocably commits all prior writes in the current transaction, so any subsequent failure cannot roll them back. `Commit()` is appropriate only at top-level entry points where partial persistence is intentional and understood.
|
||||
|
||||
See sample: `httpclient-inside-write-transaction-holds-locks.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`Modify`/`Insert` followed by `HttpClient` in the same procedure with no `Commit` between them. Detection signal: any `HttpClient` use after a write on the same execution path, especially in posting, page actions, or subscribers.
|
||||
|
||||
See sample: `httpclient-inside-write-transaction-holds-locks.bad.al`.
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50100 "IsEmpty Before FindSet Bad"
|
||||
{
|
||||
procedure ListUsCustomerNames()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields(Name);
|
||||
Customer.SetRange("Country/Region Code", 'US');
|
||||
// IsEmpty does not replace FindSet; it adds a second round-trip.
|
||||
if not Customer.IsEmpty() then
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
Message(Customer.Name);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,14 @@
|
|||
codeunit 50100 "IsEmpty Before FindSet Good"
|
||||
{
|
||||
procedure ListUsCustomerNames()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields(Name);
|
||||
Customer.SetRange("Country/Region Code", 'US');
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
Message(Customer.Name);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [isempty, findset, extra-round-trip, existence-check, false-positive]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# IsEmpty immediately before FindSet is an extra round-trip
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`IsEmpty` is the right API when the caller only needs existence — see `microsoft/knowledge/performance/use-isempty-for-existence-check.md`. It is not a cheap guard in front of a loop that will `FindSet` anyway. Both calls hit the database; `FindSet` already returns false when the filter matches nothing. Agents and reviewers often insert `if not Rec.IsEmpty() then` "for performance" and pay a second query for a result the iterator already provides.
|
||||
|
||||
## Best Practice
|
||||
|
||||
When the body iterates, open with `if Rec.FindSet() then repeat ... until Next() = 0`. Do not flag a bare `FindSet` loop as missing an `IsEmpty` precondition. Reserve `IsEmpty` for branches that never materialize the row set.
|
||||
|
||||
See sample: `isempty-before-findset-is-extra-round-trip.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`if not Rec.IsEmpty() then if Rec.FindSet() then repeat`. Also a false-positive review comment that asks to add that guard. The second read does not avoid the first; it duplicates it.
|
||||
|
||||
See sample: `isempty-before-findset-is-extra-round-trip.bad.al`.
|
||||
|
|
@ -0,0 +1,9 @@
|
|||
codeunit 50113 "Job Queue Category Bad"
|
||||
{
|
||||
procedure ConfigurePostingJobs(var PostSales: Record "Job Queue Entry"; var PostPurchases: Record "Job Queue Entry")
|
||||
begin
|
||||
// Both jobs update the same posting resources, but nothing prevents overlap.
|
||||
PostSales.Validate("Job Queue Category Code", '');
|
||||
PostPurchases.Validate("Job Queue Category Code", '');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,9 @@
|
|||
codeunit 50113 "Job Queue Category Good"
|
||||
{
|
||||
procedure ConfigurePostingJobs(var PostSales: Record "Job Queue Entry"; var PostPurchases: Record "Job Queue Entry")
|
||||
begin
|
||||
// The shared category lets only one conflicting posting job run at a time.
|
||||
PostSales.Validate("Job Queue Category Code", 'POSTING');
|
||||
PostPurchases.Validate("Job Queue Category Code", 'POSTING');
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [job-queue, category-code, concurrency, waiting, serialization, locking]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Use a job queue category to serialize conflicting jobs
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Different job queue entries can run at the same time. When two jobs update the same exclusive resource, concurrent execution can cause lock contention, deadlocks, or conflicting results. Entries with the same Job Queue Category Code are serialized: while one runs, another entry in that category waits.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Assign the same non-empty Job Queue Category Code to jobs that must not overlap, regardless of which codeunit they run. Define categories around the shared resource or exclusivity requirement, not merely around object names. Leave independent jobs in different categories so they can still run concurrently.
|
||||
|
||||
See sample: `job-queue-category-code-serializes-conflicting-jobs.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Creating or configuring multiple job queue entries that update the same exclusive resource while leaving their Job Queue Category Code empty or different. Do not flag jobs merely because they touch the same tables; the rule applies when their operation requires mutual exclusion.
|
||||
|
||||
See sample: `job-queue-category-code-serializes-conflicting-jobs.bad.al`.
|
||||
|
|
@ -0,0 +1,57 @@
|
|||
table 50112 "Queued Export Bad"
|
||||
{
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(2; Payload; Text[250])
|
||||
{
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50112 "Queued Export Worker Bad"
|
||||
{
|
||||
TableNo = "Job Queue Entry";
|
||||
|
||||
trigger OnRun()
|
||||
var
|
||||
QueuedExport: Record "Queued Export Bad";
|
||||
Client: HttpClient;
|
||||
Content: HttpContent;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
if not QueuedExport.FindFirst() then
|
||||
exit;
|
||||
|
||||
Content.WriteFrom(QueuedExport.Payload);
|
||||
Client.Post('https://example.local/exports', Content, Response);
|
||||
if not Response.IsSuccessStatusCode() then
|
||||
Error('Export failed with HTTP status %1.', Response.HttpStatusCode());
|
||||
|
||||
// If this local step fails, the external export exists but this row is retried.
|
||||
UpdateLocalStatus();
|
||||
FinalizeExport(QueuedExport);
|
||||
end;
|
||||
|
||||
local procedure UpdateLocalStatus()
|
||||
begin
|
||||
end;
|
||||
|
||||
local procedure FinalizeExport(var QueuedExport: Record "Queued Export Bad")
|
||||
begin
|
||||
QueuedExport.Delete();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,65 @@
|
|||
table 50112 "Queued Export Good"
|
||||
{
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
AutoIncrement = true;
|
||||
}
|
||||
field(2; Payload; Text[250])
|
||||
{
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
codeunit 50112 "Queued Export Worker Good"
|
||||
{
|
||||
TableNo = "Job Queue Entry";
|
||||
|
||||
trigger OnRun()
|
||||
var
|
||||
QueuedExport: Record "Queued Export Good";
|
||||
Client: HttpClient;
|
||||
Content: HttpContent;
|
||||
ContentHeaders: HttpHeaders;
|
||||
JsonPayload: JsonObject;
|
||||
RequestBody: Text;
|
||||
Response: HttpResponseMessage;
|
||||
begin
|
||||
if not QueuedExport.FindFirst() then
|
||||
exit;
|
||||
|
||||
JsonPayload.Add('idempotencyKey', Format(QueuedExport.SystemId));
|
||||
JsonPayload.Add('payload', QueuedExport.Payload);
|
||||
JsonPayload.WriteTo(RequestBody);
|
||||
|
||||
Content.WriteFrom(RequestBody);
|
||||
Content.GetHeaders(ContentHeaders);
|
||||
ContentHeaders.Clear();
|
||||
ContentHeaders.Add('Content-Type', 'application/json');
|
||||
Client.Post('https://example.local/exports', Content, Response);
|
||||
if not Response.IsSuccessStatusCode() then
|
||||
Error('Export failed with HTTP status %1.', Response.HttpStatusCode());
|
||||
|
||||
// The external service must atomically create a record only when idempotencyKey
|
||||
// does not exist. When the key already exists, it must return the existing record
|
||||
// without repeating the side effect.
|
||||
UpdateLocalStatus();
|
||||
QueuedExport.Delete();
|
||||
end;
|
||||
|
||||
local procedure UpdateLocalStatus()
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [job-queue, idempotency, retry, outbox, httpclient, external-effect]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Job queue external effects must be idempotent
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
A job queue handler can successfully create something in an external system and then fail while updating Business Central. Business Central rolls back its database changes and retries the queued work, but it cannot roll back the external request. Without a way for the external system to recognize the repeated request, the retry can create a duplicate shipment, payment, notification, or other side effect.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use a stable request ID that exists before the job queue processes the outbox row. For example, include the outbox record's `SystemId` as an `idempotencyKey` value in the JSON body of every POST attempt. The external service must enforce uniqueness on that value: when it receives the key again, it returns the existing record instead of creating another one. Delete the outbox row only after the external call and all required local updates succeed.
|
||||
|
||||
A `Processed` flag set after the external call does not solve this failure window. If a later AL error rolls back that flag, the outbox row again looks unprocessed even though the external operation already happened.
|
||||
|
||||
See sample: `job-queue-external-effects-must-be-idempotent.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Sending a state-changing request from a job queue handler with no stable request ID understood by the external API. Specifically, look for this sequence: read an outbox row, call `HttpClient.Post` or another side-effecting API, update or delete local data, and propagate an error that can cause the same outbox row to be retried. The key may be part of the request body, URI, headers, or an existing business key; a naturally idempotent remote operation is already safe and should not be flagged.
|
||||
|
||||
See sample: `job-queue-external-effects-must-be-idempotent.bad.al`.
|
||||
|
|
@ -0,0 +1,17 @@
|
|||
codeunit 50110 "Job Queue UI Bad"
|
||||
{
|
||||
TableNo = "Job Queue Entry";
|
||||
|
||||
trigger OnRun()
|
||||
begin
|
||||
if not Confirm('Process the queued export now?') then
|
||||
exit;
|
||||
|
||||
ProcessExport(Rec."Parameter String");
|
||||
Message('The queued export completed.');
|
||||
end;
|
||||
|
||||
local procedure ProcessExport(ParameterString: Text)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,14 @@
|
|||
codeunit 50110 "Job Queue UI Good"
|
||||
{
|
||||
TableNo = "Job Queue Entry";
|
||||
|
||||
trigger OnRun()
|
||||
begin
|
||||
Rec.TestField("Parameter String");
|
||||
ProcessExport(Rec."Parameter String");
|
||||
end;
|
||||
|
||||
local procedure ProcessExport(ParameterString: Text)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [job-queue, background-session, guiallowed, confirm, runmodal, client-callback]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Job queue handlers must not require user interaction
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
A job queue handler runs in a background session with no client UI. Calls that require a client callback, such as `Confirm`, `Page.RunModal`, `Report.RunModal`, upload, or download, can stop the job with a non-retriable callback error. `Message` is suppressed and logged by the server, so it cannot communicate a result to the user who scheduled the job.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Make a dedicated job queue entry point non-interactive. Validate parameters and data in AL, persist business-visible status when needed, and let failures propagate to the job queue log. If one procedure genuinely serves both foreground and background callers, isolate optional UI-only behavior behind `GuiAllowed`; do not use the guard to silently skip a decision that the operation requires.
|
||||
|
||||
See sample: `job-queue-handlers-must-not-require-ui.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `Confirm`, `Page.Run`, `Page.RunModal`, `Report.Run`, `Report.RunModal`, `Hyperlink`, `File.Upload`, or `File.Download` from a codeunit run by the job queue. Another signal is using `Message` as the only success or failure notification: no user is attached to receive it.
|
||||
|
||||
See sample: `job-queue-handlers-must-not-require-ui.bad.al`.
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
codeunit 50111 "Job Queue Failure Bad"
|
||||
{
|
||||
TableNo = "Job Queue Entry";
|
||||
|
||||
trigger OnRun()
|
||||
begin
|
||||
if not TryProcessCustomer(Rec."Parameter String") then
|
||||
exit;
|
||||
end;
|
||||
|
||||
[TryFunction]
|
||||
local procedure TryProcessCustomer(CustomerNo: Code[20])
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.Get(CustomerNo);
|
||||
ProcessCustomer(Customer);
|
||||
end;
|
||||
|
||||
local procedure ProcessCustomer(Customer: Record Customer)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50111 "Job Queue Failure Good"
|
||||
{
|
||||
TableNo = "Job Queue Entry";
|
||||
|
||||
trigger OnRun()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.Get(Rec."Parameter String");
|
||||
ProcessCustomer(Customer);
|
||||
end;
|
||||
|
||||
local procedure ProcessCustomer(Customer: Record Customer)
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [job-queue, error-propagation, tryfunction, retry, dispatcher, job-queue-log]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Job queue handlers must propagate execution failures
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
The job queue dispatcher can mark an entry as failed, record the error, and apply its configured retry behavior only when the handler terminates with an error. A handler that catches a failed `TryFunction` or Boolean-returning operation and then returns normally reports success to the dispatcher, even though its work did not complete.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Let an error that invalidates the whole run propagate out of the job queue entry point. Add context only when it helps an operator diagnose the failure and does not expose sensitive data. Per-item failures may be collected deliberately, but the batch must persist or emit an observable aggregate outcome instead of silently treating incomplete work as success.
|
||||
|
||||
See sample: `job-queue-handlers-must-propagate-failures.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling a `TryFunction`, `Codeunit.Run`, or another Boolean-returning operation from a job queue handler and using `exit` or normal fall-through on failure without recording an intentional partial-success outcome. The dispatcher sees a successful return, so the entry's status and log do not represent the failed work and configured retries are not applied.
|
||||
|
||||
See sample: `job-queue-handlers-must-propagate-failures.bad.al`.
|
||||
|
|
@ -0,0 +1,18 @@
|
|||
codeunit 50114 "Job Queue On Hold Bad"
|
||||
{
|
||||
TableNo = "Job Queue Entry";
|
||||
|
||||
trigger OnRun()
|
||||
begin
|
||||
repeat
|
||||
if not ProcessNextBatch() then
|
||||
exit;
|
||||
Rec.Get(Rec.ID);
|
||||
until Rec.Status = Rec.Status::"On Hold";
|
||||
end;
|
||||
|
||||
local procedure ProcessNextBatch(): Boolean
|
||||
begin
|
||||
exit(false);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,50 @@
|
|||
table 50114 "Job Cancellation Control"
|
||||
{
|
||||
DataClassification = SystemMetadata;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Job Queue Entry ID"; Guid)
|
||||
{
|
||||
}
|
||||
field(2; "Stop Requested"; Boolean)
|
||||
{
|
||||
}
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Job Queue Entry ID")
|
||||
{
|
||||
Clustered = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50114 "Job Queue On Hold Good"
|
||||
{
|
||||
TableNo = "Job Queue Entry";
|
||||
|
||||
trigger OnRun()
|
||||
begin
|
||||
repeat
|
||||
if not ProcessNextBatch() then
|
||||
exit;
|
||||
until IsStopRequested(Rec.ID);
|
||||
end;
|
||||
|
||||
local procedure IsStopRequested(JobQueueEntryId: Guid): Boolean
|
||||
var
|
||||
JobCancellationControl: Record "Job Cancellation Control";
|
||||
begin
|
||||
if not JobCancellationControl.Get(JobQueueEntryId) then
|
||||
exit(false);
|
||||
|
||||
exit(JobCancellationControl."Stop Requested");
|
||||
end;
|
||||
|
||||
local procedure ProcessNextBatch(): Boolean
|
||||
begin
|
||||
exit(false);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [job-queue, on-hold, cancellation, in-process, long-running, stop-request]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Putting a job queue entry on hold does not stop its current run
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
The On Hold status prevents a job queue entry from starting again, but it does not cancel a run that is already in process. A long-running handler continues until it completes, fails, reaches a cancellation point implemented by the application, or its session is stopped externally.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Use On Hold to pause future scheduling. When a long-running operation must support graceful cancellation, store a separate application-owned stop request and check it between bounded units of work. Exit only at a point where completed work and the checkpoint are consistent; use administrative session termination only when graceful cancellation is impossible.
|
||||
|
||||
See sample: `job-queue-on-hold-does-not-stop-running-work.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Polling the job queue entry's Status field from inside its handler and expecting a change to On Hold to cancel the active run. The status controls scheduling, not cooperative cancellation, so the handler can continue processing despite the operator's action.
|
||||
|
||||
See sample: `job-queue-on-hold-does-not-stop-running-work.bad.al`.
|
||||
|
|
@ -6,8 +6,8 @@ codeunit 50100 "Item Reindex Queue"
|
|||
ReindexQueue: Codeunit "Reindex Queue";
|
||||
begin
|
||||
// Only the primary key is used in the loop body; load nothing else.
|
||||
Item.SetLoadFields("No.");
|
||||
Item.SetRange("Item Category Code", CategoryCode);
|
||||
Item.SetLoadFields("No.");
|
||||
|
||||
if Item.FindSet() then
|
||||
repeat
|
||||
|
|
|
|||
|
|
@ -0,0 +1,15 @@
|
|||
codeunit 50100 "Login Subscriber IO Bad"
|
||||
{
|
||||
[EventSubscriber(ObjectType::Codeunit, Codeunit::"System Initialization", OnAfterLogin, '', false, false)]
|
||||
local procedure OnAfterLogin()
|
||||
var
|
||||
Client: HttpClient;
|
||||
Response: HttpResponseMessage;
|
||||
GLEntry: Record "G/L Entry";
|
||||
begin
|
||||
// Blocks UI, API, and job-queue session creation until HTTP and SQL finish.
|
||||
Client.Get('https://example.local/warmup', Response);
|
||||
GLEntry.SetLoadFields("Entry No.");
|
||||
if GLEntry.FindLast() then;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,30 @@
|
|||
codeunit 50100 "Login Subscriber IO Good"
|
||||
{
|
||||
[EventSubscriber(ObjectType::Codeunit, Codeunit::"System Initialization", OnAfterLogin, '', false, false)]
|
||||
local procedure OnAfterLogin()
|
||||
var
|
||||
TaskId: Guid;
|
||||
StoredId: Text;
|
||||
begin
|
||||
// Guard to interactive sessions only; background task sessions also raise OnAfterLogin.
|
||||
if not (Session.CurrentClientType() in [ClientType::Web, ClientType::Windows, ClientType::Desktop, ClientType::Tablet, ClientType::Phone]) then
|
||||
exit;
|
||||
|
||||
// Idempotent: TaskExists requires the GUID returned by CreateTask, stored across logins.
|
||||
if IsolatedStorage.Get('LoginSyncTaskId', DataScope::Company, StoredId) then
|
||||
if Evaluate(TaskId, StoredId) then
|
||||
if TaskScheduler.TaskExists(TaskId) then
|
||||
exit;
|
||||
|
||||
TaskId := TaskScheduler.CreateTask(Codeunit::"Login Subscriber IO Work", 0, true, CompanyName(), CurrentDateTime() + 60000);
|
||||
IsolatedStorage.Set('LoginSyncTaskId', Format(TaskId), DataScope::Company);
|
||||
end;
|
||||
}
|
||||
|
||||
codeunit 50101 "Login Subscriber IO Work"
|
||||
{
|
||||
trigger OnRun()
|
||||
begin
|
||||
// Isolated from session creation: outbound I/O is safe here.
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [oncompanyopen, onafterlogin, session-start, httpclient, subscriber, login]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Session-open subscribers must not do I/O
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`OnCompanyOpen`, `OnCompanyOpenCompleted`, and `System Initialization`.OnAfterLogin run while the session is being created. The platform waits until every subscriber returns before the UI, an API call, or a background session can proceed. `HttpClient` or a heavy `FindSet` here delays **every** session type, not just the user who "opened the company". Agents still put warmup sync, license checks, and HTTP probes on these events because they look like an application startup hook.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Keep company-open subscribers to cheap in-memory work: set a flag, enqueue a job-queue entry, or `TaskScheduler.CreateTask`. Perform HTTP and large SQL after the session is running, in that background work. When the subscriber can run repeatedly, use `store-scheduled-task-id-to-avoid-duplicate-tasks.md` to avoid creating the same logical task more than once.
|
||||
|
||||
See sample: `oncompanyopen-subscribers-must-not-do-io.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
An `OnAfterLogin` / `OnCompanyOpenCompleted` subscriber that calls `HttpClient` or scans a ledger. Detection signal: `HttpClient`, `FindSet`, or `CalcFields` inside a subscriber bound to those events.
|
||||
|
||||
See sample: `oncompanyopen-subscribers-must-not-do-io.bad.al`.
|
||||
|
|
@ -0,0 +1,31 @@
|
|||
page 50100 "Cue Background Task Bad"
|
||||
{
|
||||
PageType = CardPart;
|
||||
ApplicationArea = All;
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
cuegroup(Group)
|
||||
{
|
||||
field(OpenOrders; OpenOrderCount)
|
||||
{
|
||||
Caption = 'Open Sales Orders';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var
|
||||
OpenOrderCount: Integer;
|
||||
|
||||
trigger OnOpenPage()
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
begin
|
||||
// Blocks Role Center render on an exact count of sales headers.
|
||||
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Order);
|
||||
OpenOrderCount := SalesHeader.Count();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,49 @@
|
|||
page 50100 "Cue Background Task Good"
|
||||
{
|
||||
PageType = CardPart;
|
||||
ApplicationArea = All;
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
cuegroup(Group)
|
||||
{
|
||||
field(OpenOrders; OpenOrderCount)
|
||||
{
|
||||
Caption = 'Open Sales Orders';
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
var
|
||||
OpenOrderCount: Integer;
|
||||
TaskId: Integer;
|
||||
|
||||
trigger OnAfterGetCurrRecord()
|
||||
var
|
||||
Args: Dictionary of [Text, Text];
|
||||
begin
|
||||
CurrPage.EnqueueBackgroundTask(TaskId, Codeunit::"Cue Open Order Count", Args);
|
||||
end;
|
||||
|
||||
trigger OnPageBackgroundTaskCompleted(CompletedTaskId: Integer; Results: Dictionary of [Text, Text])
|
||||
begin
|
||||
if Results.ContainsKey('Count') then
|
||||
Evaluate(OpenOrderCount, Results.Get('Count'));
|
||||
end;
|
||||
}
|
||||
|
||||
codeunit 50100 "Cue Open Order Count"
|
||||
{
|
||||
trigger OnRun()
|
||||
var
|
||||
SalesHeader: Record "Sales Header";
|
||||
Results: Dictionary of [Text, Text];
|
||||
begin
|
||||
SalesHeader.SetRange("Document Type", SalesHeader."Document Type"::Order);
|
||||
Results.Add('Count', Format(SalesHeader.CountApprox()));
|
||||
Page.SetBackgroundTaskResult(Results);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [15..]
|
||||
domain: performance
|
||||
keywords: [page-background-task, cue, rolecenter, enqueuebackgroundtask, ui-thread]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Calculate expensive cues on a page background task
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Role-center cues and CardPart totals that run `CalcFields`, scans, or HTTP on the UI thread freeze the shell until they finish. Page background tasks exist to return the page immediately and fill the number later. Enqueue mechanics, cancellation, and the read-only child session are covered in `microsoft/knowledge/ui/page-background-tasks.md`. This file is the performance trigger: a cue whose value is not needed to *open* the page must not run on the render path.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Bind the cue to a page variable, enqueue a read-only calculation from `OnAfterGetCurrRecord` (not `OnAfterGetRecord` on a list), and apply the result in `OnPageBackgroundTaskCompleted`. Show a placeholder until then.
|
||||
|
||||
See sample: `page-background-tasks-for-expensive-cues.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`CalcFields` or a ledger `Count` in `OnOpenPage` / `OnAfterGetCurrRecord` of a CueGroup CardPart with no background task. The Role Center waits on SQL the user may never look at.
|
||||
|
||||
See sample: `page-background-tasks-for-expensive-cues.bad.al`.
|
||||
|
|
@ -0,0 +1,20 @@
|
|||
codeunit 50100 "Pass Var Enumerator Bad"
|
||||
{
|
||||
procedure ListUsCustomerCities()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields(Name);
|
||||
Customer.SetRange("Country/Region Code", 'US');
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
// By-value copy: JIT on City does not update the enumerator.
|
||||
Message(Customer.Name + ' ' + CityOf(Customer));
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
|
||||
local procedure CityOf(Customer: Record Customer): Text
|
||||
begin
|
||||
exit(Customer.City);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,21 @@
|
|||
codeunit 50100 "Pass Var Enumerator Good"
|
||||
{
|
||||
procedure ListUsCustomerCities()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields(Name);
|
||||
Customer.SetRange("Country/Region Code", 'US');
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
EnsureCityLoaded(Customer);
|
||||
Message(Customer.Name + ' ' + Customer.City);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
|
||||
local procedure EnsureCityLoaded(var Customer: Record Customer)
|
||||
begin
|
||||
if not Customer.AreFieldsLoaded(Customer.City) then
|
||||
Customer.LoadFields(Customer.City);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [setloadfields, jit-load, enumerator, var-parameter, pass-by-value, next]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Pass the iterated record var so a JIT load updates the enumerator
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
A `FindSet`/`Next` loop builds an enumerator from the fields selected for load. Accessing an unloaded field triggers a JIT load. When the record is passed **by value**, the copy does not share that enumerator: the JIT loads the copy and leaves the enumerator unchanged, so **every later `Next()` JIT-loads again**. Passing `var` lets the first JIT update the enumerator. `AddLoadFields` on the original record before a by-value call is the other fix. This is independent of whether `SetLoadFields` was ordered before filters.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Helpers that read extra fields on an in-flight iterator must take the record as `var`, or the caller must `AddLoadFields` those fields before the loop. Prefer declaring the extra fields up front so no JIT is needed.
|
||||
|
||||
See sample: `pass-var-record-to-preserve-partial-load-enumerator.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
A `SetLoadFields` loop that passes the iterator by value into a helper which then reads a field that was not loaded. The first row pays one JIT; every subsequent row pays it again because the enumerator never learned the extra field.
|
||||
|
||||
See sample: `pass-var-record-to-preserve-partial-load-enumerator.bad.al`.
|
||||
|
|
@ -15,12 +15,12 @@ application-area: [all]
|
|||
|
||||
## Best Practice
|
||||
|
||||
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`).
|
||||
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 trigger code, related subscribers, security filtering, `Media`/`MediaSet`, or companion fields force row-by-row fallback (see `triggers-and-media-field-regress-modifyall.md`). A visible loop for progress UX is acceptable only when evidence shows the equivalent bulk call already executes as individual operations and the loop preserves trigger and business semantics.
|
||||
|
||||
See sample: `prefer-modifyall-over-per-row-modify.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
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.
|
||||
A loop that only assigns a constant and calls `Modify(false)` on a field with no validation side effects or bulk fallback condition. A progress dialog alone does not exempt this loop. 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`.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,9 @@
|
|||
tableextension 50100 "G/L Entry Extra Ext" extends "G/L Entry"
|
||||
{
|
||||
fields
|
||||
{
|
||||
// Stored companion columns are joined on every G/L Entry read.
|
||||
field(50100; "External Reference"; Text[50]) { }
|
||||
field(50101; "Integration Payload"; Blob) { }
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,19 @@
|
|||
table 50100 "G/L Entry Extra"
|
||||
{
|
||||
Caption = 'G/L Entry Extra';
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "Entry No."; Integer)
|
||||
{
|
||||
TableRelation = "G/L Entry"."Entry No.";
|
||||
}
|
||||
field(2; "External Reference"; Text[50]) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "Entry No.") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,29 @@
|
|||
---
|
||||
bc-version: ["23.."]
|
||||
domain: performance
|
||||
keywords: [tableextension, companion-table, gl-entry, related-table, flowfield, hot-table]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Prefer a related table over stored fields on hot ledgers
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Since v23, all extensions on the same base table share at most one companion-table join, and the platform automatically excludes that join on List, ListPart, and OData pages when partial records are in effect and no extension field is loaded. However, the join is still paid on every posting path and any AL code that accesses an extension field — or that runs without partial-record semantics. On hot tables — G/L Entry, Item Ledger Entry, Cust. Ledger Entry — even a single access per posted row adds up at volume. A related table keyed by the ledger `Entry No.`, optionally surfaced with a FlowField or FactBox, leaves the base read path entirely untouched. Agents extend G/L Entry because it is "where the posting already is".
|
||||
|
||||
## Best Practice
|
||||
|
||||
Put optional, sparse, or integration attributes in a related table with the ledger entry number as primary key. Show them from a FactBox or a FlowField.
|
||||
Use a tableextension stored field only when the value must appear as a native list column and is read on almost every access.
|
||||
|
||||
See sample: `prefer-related-table-over-extension-on-hot-ledgers.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`tableextension` on `"G/L Entry"` (or another posting table) that adds several stored `Text`/`Blob` fields used only by one integration. The companion join is paid on every posting and on any AL code path that loads extension fields, even when those columns are not needed for the current operation.
|
||||
|
||||
See sample: `prefer-related-table-over-extension-on-hot-ledgers.bad.al`.
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
query 50100 "Query Bypass PK Cache Bad Q"
|
||||
{
|
||||
QueryType = Normal;
|
||||
|
||||
elements
|
||||
{
|
||||
dataitem(Customer; Customer)
|
||||
{
|
||||
filter(NoFilter; "No.") { }
|
||||
column(Name; Name) { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
codeunit 50100 "Query Bypass PK Cache Bad"
|
||||
{
|
||||
procedure CustomerName(CustomerNo: Code[20]): Text
|
||||
var
|
||||
CustomerByNo: Query "Query Bypass PK Cache Bad Q";
|
||||
begin
|
||||
// Query Open/Read never hits the server PK cache.
|
||||
CustomerByNo.SetRange(NoFilter, CustomerNo);
|
||||
CustomerByNo.Open();
|
||||
if CustomerByNo.Read() then
|
||||
exit(CustomerByNo.Name);
|
||||
CustomerByNo.Close();
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,12 @@
|
|||
codeunit 50100 "Query Bypass PK Cache Good"
|
||||
{
|
||||
procedure CustomerName(CustomerNo: Code[20]): Text
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
// Repeated Get of the same No. is served from the transaction PK cache.
|
||||
Customer.SetLoadFields(Name);
|
||||
if Customer.Get(CustomerNo) then
|
||||
exit(Customer.Name);
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [query, primary-key-cache, get, false-positive, n-plus-one, record-cache]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Query results bypass the primary-key cache
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
The Business Central server caches primary-key `Get` calls within a transaction. Query objects do not use that cache: every `Open`/`Read` goes to SQL. `avoid-get-inside-loop-on-large-table.md` is right when an unbounded inner `Get`/`FindFirst` joins two large sets. It is wrong as a blanket rewrite of repeated `Get` on the same keys. Replacing a cached `Get` with a Query that re-executes per call can be slower. This file exists so reviewers stop treating every `Get` inside a loop as a Query candidate.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Keep `Record.Get` for repeated lookups of the same primary keys in one transaction. Use a Query when the work is a true join or aggregation that the record API would express as nested scans. Do not flag a guarded `Get` on a repeating key as an N+1 solely because a Query could express the same columns.
|
||||
|
||||
See sample: `query-results-bypass-primary-key-cache.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Rewriting a helper that `Get`s Customer by `No.` on every sales line into a Query opened inside that helper. Distinct line customers still need a lookup; repeating customers were already served from the PK cache. The Query pays SQL every time.
|
||||
|
||||
See sample: `query-results-bypass-primary-key-cache.bad.al`.
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50100 "Reset Clears LoadFields Bad"
|
||||
{
|
||||
procedure ListUsCustomerNames()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields(Name);
|
||||
// Reset restores a full-row load; the SetLoadFields above is discarded.
|
||||
Customer.Reset();
|
||||
Customer.SetRange("Country/Region Code", 'US');
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
Message(Customer.Name);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
codeunit 50100 "Reset Clears LoadFields Good"
|
||||
{
|
||||
procedure ListUsCustomerNames()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.Reset();
|
||||
Customer.SetLoadFields(Name);
|
||||
Customer.SetRange("Country/Region Code", 'US');
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
Message(Customer.Name);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [reset, setloadfields, partial-record, load-selection, findset]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Reset and empty SetLoadFields restore a full-row load
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`SetLoadFields(...)` sticks to the record variable until something clears it. `Reset()` "changes fields select for loading back to all", and `SetLoadFields()` with no arguments does the same. A later `FindSet` or `Get` then materializes every normal field. Agents often place `SetLoadFields` first, then `Reset` to apply new filters, and assume the partial selection survives. It does not.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Call `Reset` (or empty `SetLoadFields()`) first when the variable must be reused, then call `SetLoadFields` with the fields the next read actually uses, then apply filters and read. After `Reset`, a new `SetLoadFields` is required; the previous list is gone.
|
||||
|
||||
See sample: `reset-clears-partial-record-selection.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`SetLoadFields(...)` followed by `Reset()` (or by parameterless `SetLoadFields()`) and then `FindSet` without restoring the load list. The filters look correct; the SQL still selects every column.
|
||||
|
||||
See sample: `reset-clears-partial-record-selection.bad.al`.
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50100 "Skip LoadFields Write Bad"
|
||||
{
|
||||
procedure CopyActiveCustomers(var TempCustomer: Record Customer temporary)
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
// TransferFields requires all fields; partial load forces JIT per row.
|
||||
Customer.SetLoadFields("No.", Name);
|
||||
Customer.SetRange(Blocked, Customer.Blocked::" ");
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
TempCustomer.TransferFields(Customer);
|
||||
TempCustomer.Insert();
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
codeunit 50100 "Skip LoadFields Write Good"
|
||||
{
|
||||
procedure CopyActiveCustomers(var TempCustomer: Record Customer temporary)
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
// TransferFields needs all fields; omit SetLoadFields so the initial read loads the full row.
|
||||
Customer.SetRange(Blocked, Customer.Blocked::" ");
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
TempCustomer.TransferFields(Customer);
|
||||
TempCustomer.Insert();
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [setloadfields, partial-record, jit-load, modify, insert, transferfields, write-path]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Skip SetLoadFields on write and copy paths
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`SetLoadFields` is a read optimization. The platform's [partial-record usage guidelines](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/devenv-partial-records#usage-guidelines) list the operations that require every field to already be present: `Insert`, `Delete`, `Rename`, `TransferFields`, and copying a record into a temporary table. When those operations run on a partial record, the platform issues a just-in-time load of the missing fields. That extra round-trip costs more than loading the full row on the original `FindSet` or `Get`. Note: `Modify` itself is **not** in this list — a `Modify(false)` that only touches loaded fields is safe with a partial record.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Omit `SetLoadFields` on loops whose body performs a documented full-load operation (`Insert`, `Delete`, `Rename`, `TransferFields`, or assignment into a temporary record) on the same record variable, so the initial read already materializes every field those operations need.
|
||||
|
||||
See sample: `skip-setloadfields-on-write-and-transferfields.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `SetLoadFields` immediately before a `FindSet` whose body performs `Delete`, `Rename`, `TransferFields`, or copies the record into a temporary table. The review signal is a partial-record setup on a record variable that feeds one of these documented full-load operations in the same iteration.
|
||||
|
||||
See sample: `skip-setloadfields-on-write-and-transferfields.bad.al`.
|
||||
|
|
@ -0,0 +1,15 @@
|
|||
codeunit 50115 "Scheduled Task Duplicate Bad"
|
||||
{
|
||||
procedure EnsureCleanupTask()
|
||||
begin
|
||||
// Every call creates another task for the same cleanup work.
|
||||
TaskScheduler.CreateTask(Codeunit::"Scheduled Cleanup Work Bad", 0, true, CompanyName());
|
||||
end;
|
||||
}
|
||||
|
||||
codeunit 50116 "Scheduled Cleanup Work Bad"
|
||||
{
|
||||
trigger OnRun()
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,23 @@
|
|||
codeunit 50115 "Scheduled Task Duplicate Good"
|
||||
{
|
||||
procedure EnsureCleanupTask()
|
||||
var
|
||||
TaskId: Guid;
|
||||
StoredTaskId: Text;
|
||||
begin
|
||||
if IsolatedStorage.Get('CleanupTaskId', DataScope::Company, StoredTaskId) then
|
||||
if Evaluate(TaskId, StoredTaskId) then
|
||||
if TaskScheduler.TaskExists(TaskId) then
|
||||
exit;
|
||||
|
||||
TaskId := TaskScheduler.CreateTask(Codeunit::"Scheduled Cleanup Work Good", 0, true, CompanyName());
|
||||
IsolatedStorage.Set('CleanupTaskId', Format(TaskId), DataScope::Company);
|
||||
end;
|
||||
}
|
||||
|
||||
codeunit 50116 "Scheduled Cleanup Work Good"
|
||||
{
|
||||
trigger OnRun()
|
||||
begin
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [task-scheduler, scheduled-task, taskexists, duplicate-task, createtask, guid]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Store the scheduled task ID to avoid duplicate tasks
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
Every call to `TaskScheduler.CreateTask` creates a new scheduled task and returns its unique GUID. Repeating setup or lifecycle code without retaining that GUID can create multiple tasks for the same logical work, consuming scheduler capacity and running the work more than once.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Persist the GUID returned by `CreateTask` at the same scope as the logical task. Before creating a replacement, parse the stored GUID and call `TaskScheduler.TaskExists`; create and store a new task only when the previous task no longer exists. `TaskExists` checks one GUID, not whether an equivalent codeunit is already scheduled, so callers that can schedule concurrently still need serialization around this check-and-create sequence.
|
||||
|
||||
See sample: `store-scheduled-task-id-to-avoid-duplicate-tasks.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Calling `TaskScheduler.CreateTask` every time initialization, login, setup, or another repeatable path runs while ignoring its return value. Each invocation creates another independent task even when an equivalent task is already pending.
|
||||
|
||||
See sample: `store-scheduled-task-id-to-avoid-duplicate-tasks.bad.al`.
|
||||
|
|
@ -1,7 +1,7 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [modifyall, deleteall, regression, triggers, media, getglobaltabletriggermask, subscriber]
|
||||
keywords: [modifyall, deleteall, regression, triggers, media, security-filtering, companion-fields, subscriber, progress]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
|
|
@ -11,12 +11,12 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
`ModifyAll` and `DeleteAll` usually execute as single SQL statements, but the platform falls back to a fetch-then-row-by-row loop under specific conditions. Per the upstream guidance, the regression is triggered by any of: global database triggers defined via `GetGlobalTableTriggerMask` or `GetDatabaseTableTriggerSetup` (so that `OnDatabaseDelete`/`OnGlobalDelete` must run); event subscribers on the table's `OnBeforeDelete`/`OnAfterDelete` (for `DeleteAll`) or `OnBeforeModify`/`OnAfterModify` (for `ModifyAll`); or "adding a Media or MediaSet table field to either the table or table extension." Each of these forces the platform to materialize each affected row in AL.
|
||||
`ModifyAll` and `DeleteAll` can limit SQL calls, but Microsoft documents that they [revert to individual calls](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/optimize-sql-al-database-methods-and-performance-on-server#modifyall-and-deleteall) when the table has trigger code, related modify/delete/global/database event subscribers, active security filtering, `Media` or `MediaSet` fields, or fields added through companion tables. These conditions must be assessed from the target table and runtime context, not only from the visible bulk call.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Before introducing any of the above on a table — a global trigger registration, a `Modify`/`Delete` subscriber, a media or media-set field — note every `ModifyAll`/`DeleteAll` that targets the table and assess whether the regression cost is acceptable. The upstream guidance is explicit: "There should be a very good reason for doing any of the above since they will significantly regress performance of `ModifyAll` and/or `DeleteAll`." Once a table has regressed, multiple `ModifyAll` calls each iterate the rows themselves, so consolidating to one explicit `FindSet`+`Modify` loop becomes faster than chaining several `ModifyAll` calls.
|
||||
Before introducing a fallback condition, audit the `ModifyAll`/`DeleteAll` call sites that target the table and assess the regression cost. Once a bulk path already executes row by row, one explicit loop can be reasonable when it preserves the same trigger semantics and adds required per-row progress UX; consolidating several regressed bulk calls into one pass can also avoid repeated iteration. This is a narrow equivalence check, not a generic progress-dialog exemption: when no fallback condition applies, retain the bulk API.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
Adding a media field to a hot table — or subscribing to its modify/delete events from a generic logging codeunit — without auditing the bulk-write call sites. The schema change is mechanical; the performance change is invisible at the call site and only surfaces when a previously fast `ModifyAll` starts paying the per-row trigger cost in production. The mirror anti-pattern is chaining several `ModifyAll` calls on a table that has already regressed; each one re-iterates the same rows.
|
||||
Adding a fallback condition to a hot table without auditing bulk-write call sites, or replacing a working bulk API with a per-row loop solely to show progress. The mirror anti-pattern is chaining several bulk calls on a table that already falls back, causing repeated row-by-row passes.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,60 @@
|
|||
table 50100 "Campaign Member"
|
||||
{
|
||||
Caption = 'Campaign Member';
|
||||
// Full list as lookup runs FactBoxes and extra columns on every dropdown.
|
||||
LookupPageId = Page::"Campaign Member List";
|
||||
DrillDownPageId = Page::"Campaign Member List";
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; Name; Text[100]) { }
|
||||
field(3; "Balance (LCY)"; Decimal) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
page 50100 "Campaign Member List"
|
||||
{
|
||||
PageType = List;
|
||||
SourceTable = "Campaign Member";
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
repeater(Rows)
|
||||
{
|
||||
field("No."; Rec."No.") { }
|
||||
field(Name; Rec.Name) { }
|
||||
field("Balance (LCY)"; Rec."Balance (LCY)") { }
|
||||
}
|
||||
}
|
||||
area(factboxes)
|
||||
{
|
||||
// Full list carries this FactBox on every dropdown open — expensive.
|
||||
part(Details; "Campaign Member Details FB") { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
page 50101 "Campaign Member Details FB"
|
||||
{
|
||||
PageType = CardPart;
|
||||
SourceTable = "Campaign Member";
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
field("No."; Rec."No.") { }
|
||||
field(Name; Rec.Name) { }
|
||||
field("Balance (LCY)"; Rec."Balance (LCY)") { }
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,57 @@
|
|||
table 50100 "Campaign Member"
|
||||
{
|
||||
Caption = 'Campaign Member';
|
||||
LookupPageId = Page::"Campaign Member Lookup";
|
||||
DrillDownPageId = Page::"Campaign Member List";
|
||||
DataClassification = CustomerContent;
|
||||
|
||||
fields
|
||||
{
|
||||
field(1; "No."; Code[20]) { }
|
||||
field(2; Name; Text[100]) { }
|
||||
field(3; "Balance (LCY)"; Decimal) { }
|
||||
}
|
||||
|
||||
keys
|
||||
{
|
||||
key(PK; "No.") { Clustered = true; }
|
||||
}
|
||||
}
|
||||
|
||||
page 50100 "Campaign Member Lookup"
|
||||
{
|
||||
PageType = List;
|
||||
SourceTable = "Campaign Member";
|
||||
Caption = 'Campaign Members';
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
repeater(Rows)
|
||||
{
|
||||
field("No."; Rec."No.") { }
|
||||
field(Name; Rec.Name) { }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
page 50101 "Campaign Member List"
|
||||
{
|
||||
PageType = List;
|
||||
SourceTable = "Campaign Member";
|
||||
|
||||
layout
|
||||
{
|
||||
area(content)
|
||||
{
|
||||
repeater(Rows)
|
||||
{
|
||||
field("No."; Rec."No.") { }
|
||||
field(Name; Rec.Name) { }
|
||||
field("Balance (LCY)"; Rec."Balance (LCY)") { }
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [lookuppageid, lookup-page, list-page, factbox, table-relation, dropdown]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Point lookups at a dedicated lookup page, not the full list
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
A `TableRelation` lookup opens the table's `LookupPageId`. If that is the full list page, the lookup runs that page's triggers, FactBoxes, and calculated fields even though the dropdown never shows them. The base application added dedicated Customer, Vendor, and Item lookup pages for this reason. Agents set `LookupPageId` to the main list because it already exists.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Give master tables a slim lookup page (`PageType = List`, few columns, no FactBoxes, no heavy `OnAfterGetRecord`) and assign it to `LookupPageId`. Keep the full list for `DrillDownPageId` and the role-explorer entry.
|
||||
|
||||
See sample: `use-dedicated-lookup-pages-not-full-lists.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`LookupPageId = Page::"... List"` on a table that already has (or should have) a lookup page. Opening a field lookup then pays list-page cost. The signal is `LookupPageId` pointing at a page that declares FactBoxes or a wide repeater.
|
||||
|
||||
See sample: `use-dedicated-lookup-pages-not-full-lists.bad.al`.
|
||||
|
|
@ -1,7 +1,7 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [deleteall, bulk-delete, sql, ondelete, trigger-bypass]
|
||||
keywords: [deleteall, bulk-delete, sql, ondelete, trigger-bypass, security-filtering, media, companion-fields]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
|
|
@ -13,16 +13,16 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
`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.
|
||||
`DeleteAll(false)` is eligible for a set-based SQL delete with the record variable's filters applied, but it is not guaranteed to stay one statement. Microsoft documents that `DeleteAll` [reverts to individual calls](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/optimize-sql-al-database-methods-and-performance-on-server#modifyall-and-deleteall) when the table has trigger code, related delete/global/database event subscribers, active security filtering, `Media` or `MediaSet` fields, or fields added through companion tables. Setting `RunTrigger` to false skips the base table `OnDelete` trigger, but [table-extension `OnBeforeDelete` and `OnAfterDelete` triggers still run](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/developer/methods-auto/record/record-deleteall-method#remarks).
|
||||
|
||||
## Best Practice
|
||||
|
||||
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.
|
||||
Use filtered `DeleteAll(false)` for purpose-built staging or cleanup tables only after verifying that base-table `OnDelete` logic is unnecessary and that trigger code, related subscribers, security filtering, media fields, and companion 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`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
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.
|
||||
Iterating with `FindSet` + `Delete(false)` to clear a filtered staging batch that has no delete logic or fallback condition. The reverse mistake is assuming `DeleteAll` is always one SQL statement without checking the documented fallback conditions.
|
||||
|
||||
See sample: `use-deleteall-for-filtered-bulk-deletion.bad.al`.
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ codeunit 50491 "Perf AutoCalcFields Bad"
|
|||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
Customer.CalcFields("Balance (LCY)");
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ codeunit 50490 "Perf AutoCalcFields Good"
|
|||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
Customer.SetFilter("Credit Limit (LCY)", '>0');
|
||||
Customer.SetLoadFields("Credit Limit (LCY)");
|
||||
Customer.SetAutoCalcFields("Balance (LCY)");
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
|
|
|
|||
|
|
@ -4,8 +4,8 @@ codeunit 50218 "Perf Sample LoadFields Good"
|
|||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields(Name);
|
||||
Customer.SetRange("Country/Region Code", 'US');
|
||||
Customer.SetLoadFields(Name);
|
||||
if Customer.FindSet() then
|
||||
repeat
|
||||
Message(Customer.Name);
|
||||
|
|
|
|||
|
|
@ -1,7 +1,7 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [setloadfields, partial-record, normal-field, flowfield, get, findset]
|
||||
keywords: [setloadfields, partial-record, normal-field, flowfield, get, findset, statement-order]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
|
|
@ -11,11 +11,11 @@ application-area: [all]
|
|||
|
||||
## Description
|
||||
|
||||
`SetLoadFields(...)` declares the subset of normal fields the next read should materialize, "reducing data read and transfer thereby improving performance significantly." Per the upstream guidance, "the gains scale with the amount of rows read, so for loops that read many rows `SetLoadFields` is even more important." Primary-key fields, `SystemId`, and system audit fields are loaded automatically, "and fields that are filtered on are also automatically included" — those do not need to appear in the list. `SetLoadFields` only affects `FieldClass = Normal`; it does not narrow FlowFields or FlowFilters.
|
||||
`SetLoadFields(...)` declares the subset of normal fields the next read should materialize, "reducing data read and transfer thereby improving performance significantly." Per the upstream guidance, "the gains scale with the amount of rows read, so for loops that read many rows `SetLoadFields` is even more important." Primary-key fields, `SystemId`, and system audit fields are loaded automatically, "and fields that are filtered on are also automatically included" — those do not need to appear in the list. `SetLoadFields` only affects `FieldClass = Normal`; it does not narrow FlowFields or FlowFilters. Its position relative to `SetRange`/`SetFilter` does not change the projection: filtered fields are added to the load set at read time either way. Projection-changing operations are separate: `AddLoadFields(...)` expands the selection, a later `SetLoadFields(...)` or `SetBaseLoadFields()` overwrites it, and `Reset()` or a fieldless `SetLoadFields()` restores all readable normal fields.
|
||||
|
||||
## Best Practice
|
||||
|
||||
Before a `Get`, `FindSet`, or `FindFirst` that the procedure follows by reading only a handful of the table's fields, call `SetLoadFields` listing exactly those fields. The pattern `SetLoadFields(...); if Record.Get(...) then ...` is the upstream-endorsed shape. Skip `SetLoadFields` when the table has few fields (under ten), when the code reads most of them (above 60 %), when the loop runs ten or fewer iterations, or when the table is exempt for other reasons (`singleton-setup-tables-need-no-access-optimization.md`, `temporary-tables-have-no-database-cost.md`). For report dataitems, use `AddLoadFields` in `OnPreDataItem` instead (see `addloadfields-in-report-onpredataitem.md`).
|
||||
Before a `Get`, `FindSet`, or `FindFirst` that the procedure follows by reading only a handful of the table's fields, call `SetLoadFields` listing exactly those fields. The pattern `SetLoadFields(...); if Record.Get(...) then ...` is the upstream-endorsed shape. Place the call immediately before the read, after any `SetRange`/`SetFilter`, so a reader can see at a glance which read the selection governs and any projection-changing operation is easy to spot. Skip `SetLoadFields` when the table has few fields (under ten), when the code reads most of them (above 60 %), when the loop runs ten or fewer iterations, or when the table is exempt for other reasons (`singleton-setup-tables-need-no-access-optimization.md`, `temporary-tables-have-no-database-cost.md`). For report dataitems, use `AddLoadFields` in `OnPreDataItem` instead (see `addloadfields-in-report-onpredataitem.md`).
|
||||
|
||||
See sample: `use-setloadfields-for-partial-records.good.al`.
|
||||
|
||||
|
|
@ -23,4 +23,6 @@ See sample: `use-setloadfields-for-partial-records.good.al`.
|
|||
|
||||
Loading a wide table and reading one field per row in a loop. The bytes transferred per row are dominated by the columns the procedure does not touch; the SQL query selects them anyway. The same applies to a single `Get` on a wide table — the platform reads the whole row when a single field would have sufficed.
|
||||
|
||||
Statement order is not part of this anti pattern. `SetLoadFields` placed ahead of `SetRange`/`SetFilter` materializes exactly the same columns as the reverse order, so a reviewer reports it as a readability observation at most — never as a performance defect.
|
||||
|
||||
See sample: `use-setloadfields-for-partial-records.bad.al`.
|
||||
|
|
|
|||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50100 "Validate Partial Rec Bad"
|
||||
{
|
||||
procedure UppercaseUsCustomerNames()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
Customer.SetLoadFields(Name);
|
||||
Customer.SetRange("Country/Region Code", 'US');
|
||||
if Customer.FindSet(true) then
|
||||
repeat
|
||||
// Validate touches other fields and TableRelation reads; JIT undoes the partial load.
|
||||
Customer.Validate(Name, UpperCase(Customer.Name));
|
||||
Customer.Modify(false);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,16 @@
|
|||
codeunit 50100 "Validate Partial Rec Good"
|
||||
{
|
||||
procedure UppercaseUsCustomerNames()
|
||||
var
|
||||
Customer: Record Customer;
|
||||
begin
|
||||
// Include every field that Name.OnValidate reads so the runtime never JIT-loads.
|
||||
Customer.SetLoadFields(Name, "Search Name");
|
||||
Customer.SetRange("Country/Region Code", 'US');
|
||||
if Customer.FindSet(true) then
|
||||
repeat
|
||||
Customer.Validate(Name, UpperCase(Customer.Name));
|
||||
Customer.Modify(false);
|
||||
until Customer.Next() = 0;
|
||||
end;
|
||||
}
|
||||
|
|
@ -0,0 +1,28 @@
|
|||
---
|
||||
bc-version: [all]
|
||||
domain: performance
|
||||
keywords: [validate, setloadfields, jit-load, table-relation, onvalidate, partial-record]
|
||||
technologies: [al]
|
||||
countries: [w1]
|
||||
application-area: [all]
|
||||
---
|
||||
|
||||
# Validate on a partial record forces JIT loads
|
||||
|
||||
> Contributions welcome — open a PR to refine or extend this article.
|
||||
|
||||
## Description
|
||||
|
||||
`Validate` runs the field's `OnValidate` trigger and TableRelation lookups. Those code paths routinely touch other fields on the same record. On a partial row those extra fields are not loaded, so the platform JIT-loads them — often the rest of the row — plus any related-table reads the trigger performs. Distinct from `skip-setloadfields-on-write-and-transferfields.md`: the write may be `Modify(false)`; `Validate` is what blows the partial load. Agents that combine `SetLoadFields` with `Validate` in a loop produce slower code than an unoptimized assignment.
|
||||
|
||||
## Best Practice
|
||||
|
||||
In a partial-record loop, assign fields directly when trigger side effects are not required. If `Validate` is required, do not use `SetLoadFields` on that iterator, or `AddLoadFields` every field the validate path can touch before the read.
|
||||
|
||||
See sample: `validate-on-partial-record-forces-jit.good.al`.
|
||||
|
||||
## Anti Pattern
|
||||
|
||||
`SetLoadFields` on a handful of columns, then `Validate` inside the loop. The load list looks optimal; runtime JIT and TableRelation I/O dominate. The signal is `Validate(` on a record that still has a `SetLoadFields` in the same procedure.
|
||||
|
||||
See sample: `validate-on-partial-record-forces-jit.bad.al`.
|
||||
Loading…
Add table
Add a link
Reference in a new issue