Microsoft/bcquality
1
0
Fork 0
mirror of https://github.com/microsoft/BCQuality.git synced 2026-10-05 14:46:55 +01:00
Code Issues Projects Releases Packages Wiki Activity Actions 4

Address review feedback on performance guidance

Browse source 
Clarify predicate-supporting keys versus covering queries, demonstrate proven cache reuse, and evaluate the updated partial-load and bulk-update rules.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
This commit is contained in:
Jesper Schulz-Wedde 2026-09-28 12:19:34 +02:00
parent af94c135ad
commit 821d62f16f
8 changed files with 65 additions and 32 deletions
Download patch file Download diff file Expand all files Collapse all files

22
microsoft/knowledge/performance/cache-repeated-filtered-results-with-explicit-scope.bad.al
View file

@ -1,18 +1,24 @@
codeunit 50363 "Perf Variant Cache Bad"
{
procedure CountLinesWithVariants(OrderNo: Code[20]) VariantLines: Integer
procedure CountAndCollectVariantLines(var TempSalesLine: Record "Sales Line" temporary; OrderNo: Code[20]; var VariantItemNos: List of [Code[20]]) VariantLines: Integer
var
SalesLine: Record "Sales Line";
ItemVariant: Record "Item Variant";
begin
SalesLine.SetRange("Document Type", SalesLine."Document Type"::Order);
SalesLine.SetRange("Document No.", OrderNo);
SalesLine.SetRange(Type, SalesLine.Type::Item);
if SalesLine.FindSet() then
TempSalesLine.SetRange("Document Type", TempSalesLine."Document Type"::Order);
TempSalesLine.SetRange("Document No.", OrderNo);
TempSalesLine.SetRange(Type, TempSalesLine.Type::Item);
if TempSalesLine.FindSet() then
repeat
ItemVariant.SetRange("Item No.", SalesLine."No.");
ItemVariant.SetRange("Item No.", TempSalesLine."No.");
if not ItemVariant.IsEmpty() then
VariantLines += 1;
until SalesLine.Next() = 0;
until TempSalesLine.Next() = 0;
if TempSalesLine.FindSet() then
repeat
ItemVariant.SetRange("Item No.", TempSalesLine."No.");
if not ItemVariant.IsEmpty() then
VariantItemNos.Add(TempSalesLine."No.");
until TempSalesLine.Next() = 0;
end;
}

52
microsoft/knowledge/performance/cache-repeated-filtered-results-with-explicit-scope.good.al
View file

@ -1,24 +1,48 @@
codeunit 50363 "Perf Variant Cache Good"
{
procedure CountLinesWithVariants(OrderNo: Code[20]) VariantLines: Integer
procedure CountAndCollectVariantLines(var TempSalesLine: Record "Sales Line" temporary; OrderNo: Code[20]; var VariantItemNos: List of [Code[20]]) VariantLines: Integer
var
SalesLine: Record "Sales Line";
ItemVariant: Record "Item Variant";
HasVariantsByItem: Dictionary of [Code[20], Boolean];
HasVariants: Boolean;
begin
SalesLine.SetRange("Document Type", SalesLine."Document Type"::Order);
SalesLine.SetRange("Document No.", OrderNo);
SalesLine.SetRange(Type, SalesLine.Type::Item);
if SalesLine.FindSet() then
TempSalesLine.SetRange("Document Type", TempSalesLine."Document Type"::Order);
TempSalesLine.SetRange("Document No.", OrderNo);
TempSalesLine.SetRange(Type, TempSalesLine.Type::Item);
if TempSalesLine.FindSet() then
repeat
if not HasVariantsByItem.Get(SalesLine."No.", HasVariants) then begin
ItemVariant.SetRange("Item No.", SalesLine."No.");
HasVariants := not ItemVariant.IsEmpty();
HasVariantsByItem.Add(SalesLine."No.", HasVariants);
end;
if HasVariants then
if HasVariants(TempSalesLine."No.", ItemVariant, HasVariantsByItem) then
VariantLines += 1;
until SalesLine.Next() = 0;
until TempSalesLine.Next() = 0;
if TempSalesLine.FindSet() then
repeat
if HasVariants(TempSalesLine."No.", ItemVariant, HasVariantsByItem) then
VariantItemNos.Add(TempSalesLine."No.");
until TempSalesLine.Next() = 0;
end;
local procedure HasVariants(ItemNo: Code[20]; var ItemVariant: Record "Item Variant"; var HasVariantsByItem: Dictionary of [Code[20], Boolean]): Boolean
var
CachedResult: Boolean;
begin
if HasVariantsByItem.Get(ItemNo, CachedResult) then
exit(CachedResult);
ItemVariant.SetRange("Item No.", ItemNo);
CachedResult := not ItemVariant.IsEmpty();
HasVariantsByItem.Add(ItemNo, CachedResult);
exit(CachedResult);
end;
procedure CountDistinctItemsWithVariants(var TempItems: Record Item temporary) VariantItems: Integer
var
ItemVariant: Record "Item Variant";
begin
if TempItems.FindSet() then
repeat
ItemVariant.SetRange("Item No.", TempItems."No.");
if not ItemVariant.IsEmpty() then
VariantItems += 1;
until TempItems.Next() = 0;
end;
}

6
microsoft/knowledge/performance/cache-repeated-filtered-results-with-explicit-scope.md
View file

@ -11,15 +11,15 @@ application-area: [all]
## Description
A loop can ask the same filtered existence or calculation question for many rows sharing a business key. Unlike repeated primary-key `Get` calls, non-keyed filtered lookups are not automatically answered by the primary-key record cache. Memoization can remove repeated AL and data-access work, but a cache keyed by too few inputs or kept past a data change returns the wrong answer.
A loop can ask the same filtered existence or calculation question for many rows sharing a business key, or two phases can ask it for the same rows. Unlike repeated primary-key `Get` calls, non-keyed filtered lookups are not automatically answered by the primary-key record cache. Memoization can remove repeated AL and data-access work, but a cache keyed by too few inputs or kept past a data change returns the wrong answer. A single pass over lines with unknown, possibly distinct item numbers does not establish reuse.
## Best Practice
First reuse an already-loaded result if valid. For a repeated, stable filtered lookup, keep a local dictionary for one operation, keyed by every input that affects the result (including company, filters, date, unit, currency, and quantity where applicable). Cache negative results as well as positive ones; distinguish a missing dictionary entry from an entry whose value is `false`. If underlying records can change during the run, update or invalidate the entry, or do not cache it. Bound entries or process in chunks when key cardinality is large. Check distinct complete keys, hits/misses, SQL work, AL time, and memory before adding a cache to a low-reuse workload. Use a temporary table for record-shaped values or multiple keys. See sample: [`cache-repeated-filtered-results-with-explicit-scope.good.al`](cache-repeated-filtered-results-with-explicit-scope.good.al).
First reuse an already-loaded result if valid. Require evidence that complete lookup keys actually repeat, such as repeated queries for the same item in two passes over an unchanged line set (as in the sample), or measured cache hits. For a repeated, stable filtered lookup, keep a local dictionary for one operation, keyed by every input that affects the result (including company, filters, date, unit, currency, and quantity where applicable). Cache negative results as well as positive ones; distinguish a missing dictionary entry from an entry whose value is `false`. If underlying records can change during the run, update or invalidate the entry, or do not cache it. Bound entries or process in chunks when key cardinality is large. Check distinct complete keys, hits/misses, SQL work, AL time, and memory before adding a cache to a low-reuse workload. Use a temporary table for record-shaped values or multiple keys. See sample: [`cache-repeated-filtered-results-with-explicit-scope.good.al`](cache-repeated-filtered-results-with-explicit-scope.good.al).
## Anti Pattern
Running the same filtered `IsEmpty` for every line with a repeated item, or caching a price by item alone when customer, variant, date, and quantity affect it. Do **not** infer one SQL round trip from each `Get` inside a loop or automatically wrap a cached primary-key read in another dictionary; see [primary-key cache exceptions](primary-key-get-in-loop-is-transaction-cached.md). See sample: [`cache-repeated-filtered-results-with-explicit-scope.bad.al`](cache-repeated-filtered-results-with-explicit-scope.bad.al).
Running the same filtered `IsEmpty` in two passes over the same unchanged lines (even when every item number is distinct within a pass), or caching a price by item alone when customer, variant, date, and quantity affect it. Do **not** infer a cache opportunity from a single pass with no established key reuse, such as visiting each distinct Item once; the [good sample](cache-repeated-filtered-results-with-explicit-scope.good.al) also shows this valid direct lookup. Do not equate each `Get` with a SQL round trip or automatically wrap a cached primary-key read in another dictionary; see [primary-key cache exceptions](primary-key-get-in-loop-is-transaction-cached.md). See sample: [`cache-repeated-filtered-results-with-explicit-scope.bad.al`](cache-repeated-filtered-results-with-explicit-scope.bad.al).
## References

1
microsoft/knowledge/performance/design-covering-keys-from-read-pattern.good.al
View file

@ -17,6 +17,7 @@ table 50360 "Perf Read Entry"
key(PK; "Entry No.") { Clustered = true; }
key(ByCustomerDate; "Customer No.", "Posting Date")
{
// Supports the filters and explicit payload; implicit system fields may still require lookups.
IncludedFields = "Item No.", Quantity;
}
}

8
microsoft/knowledge/performance/design-covering-keys-from-read-pattern.md
View file

@ -7,21 +7,21 @@ countries: [w1]
application-area: [all]
---
# Design a covering key from the measured read pattern
# Design a read-pattern key and verify whether it covers the query
## Description
A secondary key should serve a particular read, not a list of fields that happen to look important. The order of key fields affects which filters and sort orders it can support; `IncludedFields` supplies non-key payload columns without making them ordered key columns. `SetCurrentKey` sets an order, not an index hint (see [sort guidance](setcurrentkey-sets-sort-order-not-index-hint.md)). Coverage alone does not make a query selective.
A secondary key should serve a particular read, not a list of fields that happen to look important. The order of key fields affects which filters and sort orders it can support; `IncludedFields` supplies non-key payload columns without making them ordered key columns. `SetCurrentKey` sets an order, not an index hint (see [sort guidance](setcurrentkey-sets-sort-order-not-index-hint.md)). A key that supports the predicates is not necessarily a covering index, and coverage alone does not make a query selective.
## Best Practice
For a costly, frequent read, establish its equality and range filters, joins, required ordering, actual SQL projection, cardinality, and existing physical indexes. Test a key whose leading fields support the useful predicates; for example, customer equality followed by a posting-date range. On a nonclustered secondary key, consider `IncludedFields` for small payload fields read but not filtered or ordered. Check the generated SQL (including implicitly selected and extension fields) before calling an index covering, and measure reads, sorts, lookups, latency, and write overhead. The [Database Missing Indexes page](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/database-missing-indexes) supplies candidates, not a mandate to add every suggested key.
For a costly, frequent read, establish its equality and range filters, joins, required ordering, actual SQL projection, cardinality, and existing physical indexes. Test a key whose leading fields support the useful predicates; for example, customer equality followed by a posting-date range. On a nonclustered secondary key, consider `IncludedFields` for small payload fields read but not filtered or ordered. The sample's key supports customer/date filters and includes the explicit payload, but is **not demonstrated to cover the read**: Business Central also projects `SystemId` and system audit fields on partial records. Check the generated SQL, including automatically selected, clustered-key, and extension fields, against the physical index before claiming coverage. Adding more included fields has storage and write-maintenance costs; measure reads, sorts, lookups, latency, and writes before expanding it. The [Database Missing Indexes page](https://learn.microsoft.com/en-us/dynamics365/business-central/dev-itpro/administration/database-missing-indexes) supplies candidates, not a mandate to add every suggested key.
`IncludedFields` requires runtime 8.0 (BC 19) or later and cannot be set on a primary or clustered secondary key. An included field does not participate in `SetCurrentKey` matching or maintain a SIFT sum. If the item is also a selective predicate or required ordering column, evaluate it as a key field instead. Respect table-extension key field-ownership restrictions. See sample: [`design-covering-keys-from-read-pattern.good.al`](design-covering-keys-from-read-pattern.good.al).
## Anti Pattern
Adding a key on output-only fields or requesting an unrelated `SetCurrentKey` ordering to "force" the optimizer to use that key, without establishing the read's filters or validating its plan. Do not report every uncovered read as a defect: a small table, a low-frequency query, or a write-heavy table may be better without another maintained index. See sample: [`design-covering-keys-from-read-pattern.bad.al`](design-covering-keys-from-read-pattern.bad.al).
Adding a key on output-only fields or requesting an unrelated `SetCurrentKey` ordering to "force" the optimizer to use that key, without establishing the read's filters or validating its plan. Likewise, calling an index covering just because it contains the fields explicitly listed in `SetLoadFields` ignores automatically projected columns. Do not report every uncovered read as a defect: a small table, a low-frequency query, or a write-heavy table may be better without another maintained index. See sample: [`design-covering-keys-from-read-pattern.bad.al`](design-covering-keys-from-read-pattern.bad.al).
## References

2
microsoft/knowledge/performance/review-overlapping-keys-before-adding-an-index.md
View file

@ -15,7 +15,7 @@ Adding a secondary key to a frequently written table maintains another SQL index
## Best Practice
Before adding or removing a key, inventory keys from the table and installed extensions and identify each key's consumers and purpose: seek, ordering, uniqueness, or aggregation. Check `SQLIndex`, `MaintainSQLIndex`, `SumIndexFields`, and `MaintainSIFTIndex`, not only the AL key name. For *confirmed* payload-only variants on BC 19 or later, a single nonclustered key with `IncludedFields` can be a consolidation candidate (see [covering keys](design-covering-keys-from-read-pattern.md)); an included field cannot replace a key column used for ordering or a maintained SIFT sum. Measure representative read and write workloads, including periodic reports, integrations, and other companies, before and after a supported extension/schema change. Retain a rollback path for a critical reader that regresses. See sample: [`review-overlapping-keys-before-adding-an-index.good.al`](review-overlapping-keys-before-adding-an-index.good.al).
Before adding or removing a key, inventory keys from the table and installed extensions and identify each key's consumers and purpose: seek, ordering, uniqueness, or aggregation. Check `SQLIndex`, `MaintainSQLIndex`, `SumIndexFields`, and `MaintainSIFTIndex`, not only the AL key name. For *confirmed* payload-only variants on BC 19 or later, a single nonclustered key with `IncludedFields` can be a consolidation candidate (see [read-pattern key design](design-covering-keys-from-read-pattern.md)); an included field cannot replace a key column used for ordering or a maintained SIFT sum. Including the explicit payload does not prove that the resulting index covers every automatically selected field. Measure representative read and write workloads, including periodic reports, integrations, and other companies, before and after a supported extension/schema change. Retain a rollback path for a critical reader that regresses. See sample: [`review-overlapping-keys-before-adding-an-index.good.al`](review-overlapping-keys-before-adding-an-index.good.al).
## Anti Pattern

2
microsoft/skills/review/al-performance-review.md
View file

@ -47,7 +47,7 @@ Apply these targeted cues even when simple token overlap would rank the article
- Worklist `design-covering-keys-from-read-pattern.md` for a changed secondary key alongside a filtered reader of its table, and `review-overlapping-keys-before-adding-an-index.md` when added or changed keys have overlapping leading fields. A changed key alone is not a finding; read and write workloads determine whether either rule applies.
- Worklist `preserve-buffered-inserts-by-separating-target-reads.md` when a loop calls `Insert` and interleaves operations on the insert target or `Commit`. Worklist `aggregate-before-persisting-intermediate-results.md` when repeated grouping calculations and persistent intermediate summaries appear in the same processing path. Do not infer either pattern from `Insert` or `CalcSums` alone.
- Worklist `cache-repeated-filtered-results-with-explicit-scope.md` for repeated filtered lookups on the same business inputs inside a loop, and `avoid-repeating-unchanged-validation.md` for repeated `Validate` of the same field in one path. Do not worklist either from a single lookup or validation call.
- Worklist `cache-repeated-filtered-results-with-explicit-scope.md` only when the code or workload establishes repeated **complete** lookup keys (for example, querying the same filtered set in multiple passes, or measured key reuse), or shows a cache that omits result-affecting inputs. A single loop over possibly distinct keys does not establish reuse or justify a cache finding. Worklist `avoid-repeating-unchanged-validation.md` for repeated `Validate` of the same field in one path; do not worklist it from a single validation call.
- Worklist `use-setautocalcfields-for-per-row-flowfields.md` when a record loop calls `CalcFields`, or when every row reads the same FlowField for a comparison, branch, or per-record action. Also worklist `calcsums-instead-of-calcfields-in-loop.md` when the loop accumulates one set total: use `CalcSums` directly only for stored source fields, never directly on FlowFields; a FlowField total requires deriving equivalent source filters from its `CalcFormula`.
- Worklist `hidden-flowfields-still-calculate-before-bc26-opt-in.md` when a page control directly sources a FlowField and sets `Visible = false` or a visibility expression. Suppress it when the target is known to have BC26's **Calculate only visible FlowFields** feature enabled, or when the FlowField is cheap and intentionally preloaded.
- Worklist `avoid-commit-inside-loops.md` when `Commit()` is inside a record-iteration body or a checkpoint loop lacks persisted progress that excludes completed work on retry. Do not match a commit after a complete business unit when the same transaction persists a restart-safe watermark/state and errors propagate. Still match a full-tail `FindSet` with periodic commits as unbounded retrieval; restart safety does not make it `TOP X`.