Reviewing files that changed from the base of the PR and between1b90d2a and54f7c93.

• src/NLog/Internal/LogMessageTemplateFormatter.cs (3 hunks)
• src/NLog/Internal/PropertiesDictionary.cs (18 hunks)
• src/NLog/LogEventInfo.cs (7 hunks)
• src/NLog/Logger.cs (1 hunks)
• src/NLog/MessageTemplates/MessageTemplateParameters.cs (1 hunks)

Reviewing files that changed from the base of the PR and between54f7c93 andea6180a.

• src/NLog/Internal/PropertiesDictionary.cs (16 hunks)
• src/NLog/LogEventInfo.cs (7 hunks)
• tests/NLog.UnitTests/LoggerTests.cs (4 hunks)

• GitHub Check: task-list-completed

src/NLog/Internal/PropertiesDictionary.cs (2)

129-156:Double-checkResetMessageProperties +InsertMessagePropertiesIntoEmptyDictionary behavior with mixed event vs. message properties

The reset logic for message properties and theInsertMessagePropertiesIntoEmptyDictionary helper is subtle but looks sound overall (event properties preserved, old message properties removed based onIsMessageProperty, new ones inserted with optional renaming).

Two corner cases are worth re-verifying against tests:

• When old message-template parameters all haveCaptureType.Unknown,BuildEventProperties will mark them as non-message properties (IsMessageProperty = false) and (viaresetMessageProperties) can cause_messageProperties to be cleared in theEventProperties getter. LaterResetMessageProperties(null, 0) willnot remove those entries, so they effectively become permanent event properties. That’s probably desired, but it’s a behavior change compared to “pure” template properties.
• InInsertMessagePropertiesIntoEmptyDictionary, if a new message parameter collides with an existingevent property (whereIsMessageProperty == false), the catch block intentionally does nothing, so the message parameter is not added and not renamed, while the event property remains authoritative. That matches the “event properties override template properties” rule, but it relies onIsMessageProperty being correct for all existing entries.

Given how many tests inLoggerTests exercise property overriding, I’d just confirm these two flows are covered and behave as intended.

Also applies to: 158-167, 448-481

---

497-508:PropertyDictionaryEnumerator correctly respects_messagePropertiesCount andIsMessageProperty

The updatedPropertyDictionaryEnumerator fixes the earlier pattern of blindly iterating the full array:

• It gates message-parameter enumeration using_dictionary._messagePropertiesCount, so it never walks uninitialized slots in the 3-element buffer.
• When_eventProperties exists,MoveNextValidMessageParameter only yields entries whose dictionary value hasIsMessageProperty == true, andMoveNextValidEventProperty yields only non-message entries, preserving the “message properties first, then remaining event properties” contract.
• TheReset implementation resets_messagePropertiesIndex based on the current_messagePropertiesCount, so reuse after aReset() is safe.

The only behavioral nuance is that if_eventProperties is createdafter an enumerator instance is constructed, that enumerator will never see the new dictionary entries (its_eventEnumerator was captured earlier). That’s acceptable for .NET enumerator semantics (modifying a collection during enumeration is undefined) but might be worth a one-line remark if you expect concurrent modification.

Otherwise, this resolves the past “trailing nulls from array enumerator” concern.

Also applies to: 516-517, 533-534, 550-552, 591-597, 602-603, 623-624

src/NLog/LogEventInfo.cs (3)

313-321:Message/Parameters setters + reset logic: behavior looks consistent; corner cases worth noting

The interplay between:

• Message /Parameters setters,
• ResetMessageTemplateParameters(),
• andResetFormattedMessage(bool parseMessageTemplateParameters)

looks coherent:

• When there are existing parsed message-template parameters (HasMessageTemplateParameters is true) you clear them via_properties.ResetMessageProperties() and indicate that parsing should happen again.
• When properties exist but werenot provided as constructor input (MessageProperties.Length == 0 || first.CaptureType == Unknown), you allow parsing on the next change as well.
• Constructor-supplied template parameters (non-Unknown capture types) are preserved whenMessage orParameters change unless you explicitly clear/reset them.

Two edge behaviors to re-check against tests:

1. For events created with constructor-suppliedIList<MessageTemplateParameter> and later message changes, the current logic leaves the original template parameters in place (and won’t reparse). That seems intentional, but it’s a semantic guarantee now.
2. For the ≤3-inline optimization,_properties.MessageProperties.Length will be 0, soResetMessageTemplateParameters returnstrue, allowing parsing when you later changeMessage/Parameters even though the internal inline array is non-empty. This is behaviorally correct but may cause some redundant parsing.

Given how many test cases exercise message/parameter mutations, this overall design looks okay, just slightly more subtle now.

Also applies to: 327-335, 818-829, 831-844

---

414-429:Array-basedTryCreatePropertiesInternal /CreatePropertiesInternal integration withPropertiesDictionary

The migration of these helpers toMessageTemplateParameter[]? seems correct:

• New events with parsed template parameters go throughCreatePropertiesInternal(templateParameters) which constructsPropertiesDictionary(templateParameters) and letsPropertiesDictionary own_messageProperties and_messagePropertiesCount.
• Subsequent passes from the formatter with updated parameters hit theelse if (templateParameters != null) branch inTryCreatePropertiesInternal and delegate toproperties.ResetMessageProperties(templateParameters, templateParameters.Length), which keeps existing event properties and refreshes only the message-derived portion.
• ThetemplateParameters is null && HasMessageTemplateParameters branch keeps the old “lazy capture from message” semantics.

One thing to keep in mind is thatTryCreatePropertiesInternal now silently ignores calls with a non-null but empty parameter array when_properties is null (returnsnull, no dictionary created). That matches previous behavior, but if future callers rely on “at least get an empty properties dictionary when template parsing has run”, they’ll need to handle thenull return explicitly.

Overall, the integration with the newPropertiesDictionary API looks good.

Also applies to: 431-445

---

699-719:Immutability checks updated to array model; semantics preserved

IsLogEventThreadAgnosticImmutable andHasImmutableProperties have been updated to use:

• properties.MessageProperties.Length instead of a listCount, and
• the newHasImmutableProperties(PropertiesDictionary) helper that first tries the message-properties fast path then falls back to the enumerator.

Key points:

• The “all properties come from the message template” fast-path now checksproperties.Count == _parameters?.Length && properties.Count == properties.MessageProperties.Length, which aligns with the newMessageProperties semantics.
• When that doesn’t hold, you still iterate all properties (message + event) viaGetPropertyEnumerator() and runIsSafeToDeferFormatting as before.

Given the new invariants inPropertiesDictionary, this keeps the immutability logic correct; the only impact is that the trivial fast path will trigger a bit less often (notably for the ≤3 inline case). That’s a reasonable tradeoff.

Also applies to: 720-746

tests/NLog.UnitTests/LoggerTests.cs (1)

1363-1368:Updated Conditional tests to assert onException.Message look correct*

The revised assertions forConditionalTrace* /ConditionalDebug* tests:

• Switch expected strings from hard-coded full diagnostic text to interpolated strings that appendargException.Message after the formatted error description.
• Still respect culture-dependent formatting of numeric arguments (404,5 etc.) viaNLCulture.
• Keep the overall${logger}|${message}${exception:message} layout expectation intact.

The use of string interpolation plusargException.Message aligns the tests with the new behavior of the formatter and is syntactically fine (the escaped apostrophe\' is harmless, though not strictly needed). No functional test issues here.

Also applies to: 1536-1541, 1708-1713, 1881-1886

Reviewing files that changed from the base of the PR and betweenea6180a and92ffecb.

• src/NLog/Internal/PropertiesDictionary.cs (16 hunks)
• src/NLog/LogEventInfo.cs (7 hunks)
• tests/NLog.UnitTests/LoggerTests.cs (4 hunks)

• src/NLog/LogEventInfo.cs

• GitHub Check: task-list-completed
• GitHub Check: task-list-completed

tests/NLog.UnitTests/LoggerTests.cs (1)

1364-1364:LGTM - Test expectations updated for dynamic exception messages.

The test assertions now useargException.Message interpolation instead of hardcoded strings, making the tests more maintainable and resilient to exception formatting changes. This pattern is consistently applied across all ConditionalTrace and ConditionalDebug test methods.

Also applies to: 1367-1367, 1537-1537, 1540-1540, 1709-1709, 1712-1712, 1882-1882, 1885-1885

src/NLog/Internal/PropertiesDictionary.cs (2)

82-95:LGTM - Clean migration to array-based storage.

The field declarations and constructor properly transition fromIList<MessageTemplateParameter> toMessageTemplateParameter[] with explicit count tracking. The constructor correctly delegates toResetMessageProperties to ensure the array and count are initialized together.

---

497-626:LGTM - Enumerator correctly handles array bounds.

ThePropertyDictionaryEnumerator properly iterates only over the populated portion of the array (indices 0 to_messagePropertiesCount-1), avoiding the null-entry issue mentioned in past reviews. Key safety measures:

1. Index bounds are checked against_messagePropertiesCount (lines 592, 602)
2. Index is only set to values within valid range (lines 598, 606)
3. Defensive fallback to empty array handles edge cases (lines 516, 533, 550)

The complex logic at lines 595-609 correctly handles the case where the dictionary exists and might have overridden some message properties.

Reviewing files that changed from the base of the PR and between259b573 andd9b3a62.

• src/NLog/Internal/PropertiesDictionary.cs (16 hunks)
• src/NLog/LogEventInfo.cs (5 hunks)

• GitHub Check: task-list-completed

src/NLog/LogEventInfo.cs (6)

414-429:LGTM: Array-based signature change.

The method signature now acceptsMessageTemplateParameter[]? and correctly uses.Length for null-safety checks and passes the explicit count toResetMessageProperties.

---

431-445:LGTM: Constructor accepts array-based parameters.

The signature change fromIList to array is consistent with the rest of the PR. The PropertiesDictionary constructor handles the array correctly.

---

714-718:LGTM: Updated to use array Length.

The comparison correctly usesMessageProperties.Length instead ofCount for the array-based implementation.

---

720-746:Nice improvement: foreach loop for iteration.

Switching from index-based loop toforeach on line 726 is cleaner and avoids manual bounds management when iterating over all populated elements.

---

831-845:LGTM: Array Length check for template parameters.

The condition correctly usesmessageProperties.Length to check for empty arrays and properly accesses the first element'sCaptureType.

---

468-481:Verify MessageProperties behavior when array is partially filled.

Line 468 checksMessageProperties.Length > 0. However, in PropertiesDictionary.cs line 124, theMessageProperties property returns an empty array when_messagePropertiesCount < _messageProperties.Length (i.e., when the backing array is partially filled). This means:

• If there are 2 properties in a 3-element array,MessageProperties.Length is 0 (empty array returned)
• This check would fail, falling through to line 472 to re-parse the message

Confirm that this behavior is intentional and that re-parsing is acceptable in the partially-filled case, or consider using_properties._messagePropertiesCount directly.

Run this script to check how MessageTemplateParameters handles the MessageProperties property:

src/NLog/Internal/PropertiesDictionary.cs (12)

82-94:LGTM: Array-backed storage with explicit count.

The introduction ofMessageTemplateParameter[]? _messageProperties andint _messagePropertiesCount enables the optimization to avoid dictionary allocation for ≤3 properties. Constructor correctly initializes viaResetMessageProperties with the explicit count.

---

108-122:LGTM: GetEventProperties handles migration from array to dictionary.

The logic correctly checks_messagePropertiesCount for emptiness and resets the array-backed storage whenresetMessageProperties is true (lines 115-119), ensuring properties migrate to the dictionary when needed.

---

124-124:Clarify MessageProperties behavior for partially-filled arrays.

The property returns the backing array only when_messagePropertiesCount == _messageProperties.Length (fully populated), otherwise it returns an empty array. This means:

• A 3-element array with 2 properties returns an empty array
• Consumers checking.Length > 0 will see 0 even though properties exist

Verify that all consumers correctly handle this "all-or-nothing" behavior. The usage in LogEventInfo.cs line 468 would fail this check and fall back to re-parsing, which may be inefficient.

Consider documenting this behavior or exposing a MessagePropertiesCount property for consumers that need to know the actual count.

---

126-153:LGTM: ResetMessageProperties signature updated for arrays.

The method correctly acceptsMessageTemplateParameter[]? with an explicitnewMessagePropertiesCount, handles migration from old to new properties, and updates internal fields (lines 151-152).

---

155-164:LGTM: Loop bounds respect explicit count.

The iteration usesoldMessagePropertiesCount instead of array length, ensuring only valid elements are processed.

---

198-232:LGTM: Indexer and Add optimized for array-backed storage.

Both methods checkSkipDictionaryAllocation() before usingAddOrUpdateMessageProperties, avoiding dictionary allocation when the array has capacity. The fallback toGetEventProperties(true) ensures dictionary-backed storage when needed.

---

241-246:LGTM: Clear resets both dictionary and array state.

Correctly nullifies_eventProperties,_messageProperties, and resets_messagePropertiesCount to 0.

---

349-390:LGTM: Lookup uses explicit count for bounds.

TryLookupMessagePropertyValue correctly checks_messagePropertiesCount for emptiness (line 351) and uses it for loop bounds (lines 367, 378), ensuring only valid array slots are accessed.

---

421-444:LGTM: Array-based uniqueness check.

The method correctly usesparameterList.Length for bounds and iterates over array elements to verify uniqueness.

---

449-482:LGTM: Insertion handles CaptureType and name conflicts.

The method determines whether to reset message properties based onCaptureType.Unknown (lines 451-462), iterates overmessagePropertiesCount (line 464), and mutates the array when generating unique names (line 478). The logic is sound.

---

498-627:LGTM: Enumerator uses explicit count for iteration.

ThePropertyDictionaryEnumerator correctly:

• Initializes_messagePropertiesIndex based on_messagePropertiesCount > 0 (line 508)
• Uses_messagePropertiesCount for bounds checks (lines 592-593, 603)
• Accesses array elements safely within the valid range

This addresses the issue mentioned in past review comments about trailing nulls.

---

392-414:Verify thread-safety usage intent: is PropertiesDictionary expected to handle concurrent access from multiple threads on the same LogEventInfo instance?

The review identifies real synchronization gaps: line 394's lazy initialization and line 408's non-atomic increment lack protection, while line 114 usesInterlocked.CompareExchange for_eventProperties. This inconsistency is notable.

However, the codebase provides no explicit concurrency guarantees forPropertiesDictionary orLogEventInfo. TheResetMessageProperties method and other code paths lack locks entirely, suggesting single-threaded design. Yet the publicIDictionary interface andInterlocked usage at lines 114 and 436 (LogEventInfo) create ambiguity about intended thread-safety semantics.

Confirm whetherPropertiesDictionary instances should support concurrentProperties[key] mutations from multiple threads. If yes, synchronize_messageProperties consistently. If single-threaded, the concerns are moot and the review warrants dismissal.

Reviewing files that changed from the base of the PR and between26d9b6c andbf06f98.

• src/NLog/Internal/PropertiesDictionary.cs (17 hunks)
• src/NLog/LogEventInfo.cs (5 hunks)
• tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1 hunks)

• GitHub Check: task-list-completed

Reviewing files that changed from the base of the PR and betweenbf06f98 and8937426.

• src/NLog/Internal/PropertiesDictionary.cs (17 hunks)
• src/NLog/LogEventInfo.cs (5 hunks)
• tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1 hunks)

• GitHub Check: task-list-completed

src/NLog/LogEventInfo.cs (6)

414-429:LGTM!

The signature change fromIList<MessageTemplateParameter>? toMessageTemplateParameter[]? and corresponding logic updates (usingLength instead ofCount, passingtemplateParameters.Length toResetMessageProperties) align with the array-based refactor.

---

431-445:LGTM!

The signature update toMessageTemplateParameter[]? is consistent with the broader refactor.

---

468-471:LGTM!

The getter correctly usesLength for the array-basedMessageProperties.

---

715-716:LGTM!

The update to useMessageProperties.Length is correct.

---

724-727:LGTM!

The changes to useLength andforeach over the array are appropriate for the new array-based storage.

---

844-845:LGTM!

The updates to useMessageProperties.Length are correct.

src/NLog/Internal/PropertiesDictionary.cs (24)

82-83:LGTM!

The shift to array-backed storage with explicit count tracking is the foundation of the memory optimization in this PR.

---

89-95:LGTM!

The constructor signature change and initialization logic align with the array-based refactor.

---

108-108:LGTM!

TheIsEmpty check correctly uses_messagePropertiesCount.

---

110-122:LGTM!

The updated initialization logic correctly handles the promotion from array to dictionary and clears the array-backed storage when the dictionary takes ownership.

---

126-153:LGTM!

The reworkedResetMessageProperties method correctly handles the transitions between array-backed and dictionary-backed storage, including name collision checks and property removal.

---

155-164:LGTM!

The updated iteration usingoldMessagePropertiesCount is correct.

---

166-180:LGTM!

The updated initialization logic correctly uses the explicit count parameter for capacity calculations and insertion.

---

193-193:LGTM!

TheCount property correctly returns_messagePropertiesCount when using array-backed storage.

---

199-220:LGTM!

The indexer setter optimization to avoid dictionary allocation for ≤3 properties is the key memory-saving feature of this PR. The boundary checks viaSkipDictionaryAllocation correctly ensure the array has space.

---

223-232:LGTM!

TheAdd method correctly uses the same optimization path as the indexer setter, withallowUpdate=false to throw on duplicate keys.

---

408-411:LGTM!

TheSkipDictionaryAllocation guard correctly determines when array-backed storage can be used without allocating a dictionary.

---

241-246:LGTM!

TheClear method correctly resets all storage fields.

---

329-344:LGTM!

TheTryGetValue correctly delegates toTryLookupMessagePropertyValue when using array-backed storage.

---

346-387:LGTM!

The lookup logic correctly uses linear search for small arrays and promotes to dictionary for larger counts. The threshold of 10 properties balances simplicity against performance.

---

418-441:LGTM!

The uniqueness verification correctly usesLength for array iteration.

---

446-479:LGTM!

The insertion logic correctly handles name collisions and determines whether properties are true message-template parameters or event properties based onCaptureType.

---

501-506:LGTM!

The enumerator constructor correctly initializes based on_messagePropertiesCount.

---

508-523:LGTM!

TheCurrent property correctly accesses the array at the current index.

---

525-540:LGTM!

TheCurrentParameter property correctly constructsMessageTemplateParameter from both array-backed and dictionary-backed storage.

---

542-558:LGTM!

TheCurrentProperty property correctly handles both storage types.

---

562-575:LGTM!

TheMoveNext logic correctly traverses message properties first, then event properties.

---

577-585:LGTM!

The helper correctly skips message properties in the event dictionary.

---

587-612:LGTM!

TheMoveNextValidMessageParameter correctly iterates through the array-backed message properties up to_messagePropertiesCount.

---

619-623:LGTM!

TheReset method now correctly re-initializes_eventEnumerator from_dictionary._eventProperties, addressing the issue from the past review comment.

tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1)

39-272:Comprehensive benchmark coverage.

The benchmarks appropriately exercise creation, lookup, not-found lookup, and enumeration scenarios for 1–4 properties, providing the performance data that supports the PR's optimization goals.

Reviewing files that changed from the base of the PR and betweenccc371c and69d2b95.

• src/NLog/Internal/PropertiesDictionary.cs (17 hunks)
• src/NLog/LogEventInfo.cs (5 hunks)
• tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1 hunks)
• tests/NLog.UnitTests/LogMessageFormatterTests.cs (1 hunks)

• tests/NLog.UnitTests/LogMessageFormatterTests.cs

src/NLog/LogEventInfo.cs (1)

414-844:LGTM! Consistent alignment with array-backed parameter storage.

The method signature updates (lines 414, 431) and the switch fromCount toLength checks (lines 419, 469, 715, 724, 844) correctly align with the PropertiesDictionary refactoring to useMessageTemplateParameter[] with explicit count tracking. The call toResetMessageProperties (line 426) properly passes both the array and length.

src/NLog/Internal/PropertiesDictionary.cs (6)

82-95:LGTM! Core array-backed storage.

The switch fromIList<MessageTemplateParameter> toMessageTemplateParameter[] with explicit_messagePropertiesCount (lines 82-83) enables the optimization. The constructor (lines 89-95) correctly initializes viaResetMessageProperties when the array is non-empty.

---

124-124:MessageProperties guard logic is dense but correct.

The four-part guard correctly prevents exposing event properties (those withCaptureType.Unknown) as message-template parameters, addressing the concern from previous reviews. The condition ensures only fully-populated, template-derived properties are returned.

---

204-237:Indexer and Add correctly use SkipDictionaryAllocation.

Both the indexer (lines 204-225) andAdd (lines 228-237) checkSkipDictionaryAllocation() before callingAddOrUpdateMessageProperties, ensuring the small-array path is used when space remains. The fallback toGetEventProperties(true) handles dictionary promotion.

---

126-185:LGTM! State transition logic handles dual storage correctly.

ResetMessageProperties (lines 126-158) properly manages transitions between array-only and dictionary-backed storage. The helper methodsRemoveOldMessageProperties (160-169) andInitializeEventPropertiesDictionary (171-185) correctly use the explicit count parameter to operate on the populated portion of the array.

TheresetMessageProperties out parameter (lines 115-119, 182) that nulls_messageProperties during dictionary promotion is a necessary side effect of the optimization.

---

336-392:LGTM! Lookup paths correctly handle array-backed storage.

TryGetValue (336-348) properly delegates toTryLookupMessagePropertyValue when the dictionary hasn't been allocated. The linear search (lines 351-392) uses_messagePropertiesCount for bounds and includes the optimization to promote to dictionary when count exceeds 10 (line 359).

---

506-628:Enumerator correctly iterates the populated array portion.

PropertyDictionaryEnumerator uses_messagePropertiesCount throughout (lines 510, 519, 536, 553, 594-616) to iterate only the populated slots. TheReset() method (626-627) properly reinitializes both_messagePropertiesIndex and_eventEnumerator, addressing the issue from previous reviews.

tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1)

39-272:LGTM! Benchmarks correctly measure the optimization.

The 16 benchmarks comprehensively cover the key scenarios:

• Create (1-4 properties): Measures allocation + assignment cost
• Lookup (1-4 properties): Measures TryGetValue performance with 1000 iterations
• NotFoundLookup (1-4 properties): Measures miss cost when key doesn't exist
• Enumerate (1-4 properties): Measures enumerator overhead

The benchmarks properly exercise the array-backed path (1-3 properties) versus dictionary promotion (4 properties), directly supporting the PR's performance goals. The early-exit conditions prevent dead-code elimination.

Reviewing files that changed from the base of the PR and betweena9f0f23 and3fb00bd.

• src/NLog/Internal/PropertiesDictionary.cs (18 hunks)
• src/NLog/LogEventInfo.cs (5 hunks)
• tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1 hunks)
• tests/NLog.UnitTests/LogMessageFormatterTests.cs (1 hunks)

• tests/NLog.UnitTests/LogMessageFormatterTests.cs

• GitHub Check: task-list-completed

src/NLog/LogEventInfo.cs (2)

414-429:LGTM: Signature changes are consistent.

The migration fromIList<MessageTemplateParameter> toMessageTemplateParameter[] is correctly applied. The conditional checks now useLength instead ofCount, andResetMessageProperties is properly invoked with both the array and its length.

---

715-731:LGTM: Safe iteration guarded by length check.

The logic at line 715 ensures that iteration overmessageProperties at line 727 only occurs when all properties originate from the message template (properties.Count == messageProperties.Length), preventing iteration over mixed or partially-filled arrays. This is correct.

src/NLog/Internal/PropertiesDictionary.cs (6)

52-52:LGTM: Array-backed storage foundation is well-structured.

The introduction ofSmallArraySize = 3,_messageProperties array, and_messagePropertiesCount tracking provides a clean foundation for the small-array optimization. The constructors correctly initialize the array-backed storage and check againstSmallArraySize before allocating a dictionary.

Also applies to: 84-85, 91-96, 104-108, 110-110

---

128-160:LGTM: Array-to-dictionary promotion logic is sound.

ResetMessageProperties,RemoveOldMessageProperties, andInitializeEventPropertiesDictionary correctly use_messagePropertiesCount for iteration boundaries and handle the transition from array-backed to dictionary-backed storage. The capacity calculation at line 177 (messagePropertiesCount + 2) provides reasonable headroom for the promoted dictionary.

Also applies to: 162-171, 173-187

---

205-227:LGTM: Indexer and Add leverage array-backed fast path.

The indexer andAdd method correctly gate onSkipDictionaryAllocation() to use the array-backed mutation path for up to 3 properties, then fall back to dictionary allocation. This aligns with the PR's optimization goals.

Also applies to: 230-239

---

336-394:LGTM: TryGetValue and TryLookupMessagePropertyValue optimize for small arrays.

TryGetValue delegates toTryLookupMessagePropertyValue when_eventProperties is null, and the lookup correctly uses_messagePropertiesCount as the iteration boundary (lines 371, 382). The threshold check at line 361 (_messagePropertiesCount > 10) intelligently promotes to dictionary for larger counts.

---

453-486:LGTM: InsertMessagePropertiesIntoEmptyDictionary handles CaptureType.Unknown correctly.

Lines 455–466 detect when all properties haveCaptureType.Unknown (event properties added via indexer) and setresetMessageProperties to true, signaling that the array should be cleared after dictionary promotion. This prevents event properties from masquerading as message-template properties. The iteration at line 468 correctly usesmessagePropertiesCount as the boundary.

---

502-631:LGTM: PropertyDictionaryEnumerator correctly tracks array count.

The enumerator fields (lines 505–506) include_messagePropertiesIndex initialized to -1 when_messagePropertiesCount > 0 (line 512).MoveNextValidMessageParameter (lines 594–619) uses_dictionary._messagePropertiesCount as the iteration boundary (lines 596, 607), andReset (lines 626–630) properly reinitializes both_messagePropertiesIndex and_eventEnumerator.

tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (4)

41-73:LGTM: Create benchmarks correctly exercise array-backed storage and dictionary promotion.

TheCreateOneProperty,CreateTwoProperty,CreateThreeProperty, andCreateFourProperty benchmarks instantiatePropertiesDictionary and add 1–4 properties. This exercises the small-array path (1–3 properties) and the dictionary-promotion threshold (4 properties), aligning with the PR's optimization goals.

---

75-139:LGTM: Lookup benchmarks measure repeated access performance.

TheLookupOneProperty,LookupTwoProperty,LookupThreeProperty, andLookupFourProperty benchmarks add properties and loop 1000 times callingTryGetValue for existing keys. This measures the lookup performance improvements reported in the PR objectives (e.g., 30,178→5,065 ns for 1 property).

---

141-193:LGTM: NotFoundLookup benchmarks measure negative-lookup performance.

TheNotFoundLookup* benchmarks loop 1000 times searching for a missing key ("Hello5"). This exercises the early-exit paths inTryLookupMessagePropertyValue and measures the improvements noted in the PR objectives (e.g., 19,132→4,837 ns for 1 property).

---

195-271:LGTM: Enumerate benchmarks measure iteration performance.

TheEnumerate* benchmarks loop 1000 times callingGetPropertyEnumerator and iterating viaMoveNext. The null-check onCurrent.Key (lines 206, 225, 245, 266) prevents the compiler from optimizing away the loop body. These benchmarks measure the enumeration improvements reported in the PR objectives (e.g., 70,127→36,365 ns for 1 property).

Reviewing files that changed from the base of the PR and between3fb00bd andf8e3d6c.

• src/NLog/Internal/LogMessageTemplateFormatter.cs (3 hunks)
• src/NLog/Internal/PropertiesDictionary.cs (17 hunks)
• src/NLog/LogEventInfo.cs (5 hunks)
• src/NLog/Logger.cs (1 hunks)
• tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1 hunks)
• tests/NLog.UnitTests/LogMessageFormatterTests.cs (1 hunks)

• src/NLog/Logger.cs
• src/NLog/LogEventInfo.cs

• GitHub Check: task-list-completed
• GitHub Check: task-list-completed

src/NLog/Internal/LogMessageTemplateFormatter.cs (2)

138-200:Array-based Render overloads preserve semantics and avoid IList overhead

The two Render overloads correctly mirror the previous IList-based logic while switching toMessageTemplateParameter[]?:

• Positional-only templates stay on the fast path without allocating the parameter array.
• Mixed/templated cases still “rewind” and rebuild with sequentialholeIndex, and the span overload usessb.ClearBuilder() appropriately.
• Using the sameholeIndex accounting in both overloads ensuresVerifyMessageTemplateParameters sees a consistent count.

I don’t see any new correctness or allocation pitfalls here; the behavior should be equivalent with better locality.

Also applies to: 203-264

---

267-278:Truncation of message-template parameters is correct and safer

VerifyMessageTemplateParameters now trims the backing array toholeIndex, eliminating unused/default slots when there are fewer holes thanparameters.Length. This avoids leaking dummy entries intoPropertiesDictionary and keeps_messagePropertiesCount aligned with the actual data.

The implementation is straightforward and allocation is bounded by the number of real holes, so this looks solid.

tests/NLog.UnitTests/LogMessageFormatterTests.cs (1)

228-246:Good coverage of positional templates vs inline Properties storage

This test nicely validates that:

• MessageTemplateParameters for a positional template are derived from_parameters (and markedCaptureType.Normal), not fromProperties.
• InlineProperties["0".."2"] (via the small-array path) remain intact and are not treated as template metadata.

It’s a precise regression test for the new PropertiesDictionary behavior around positional placeholders.

src/NLog/Internal/PropertiesDictionary.cs (1)

502-583:Enumerator correctly interleaves message and event properties after the refactor

The updatedPropertyDictionaryEnumerator logic looks consistent and fixes earlier pitfalls:

• _messagePropertiesIndex is initialized from_messagePropertiesCount, so message-template parameters (when present) are always yielded first.
• MoveNextValidMessageParameter respects_messagePropertiesCount and, when a dictionary exists, only advances to entries whose backingPropertyValue is markedIsMessageProperty, avoiding duplication with the event-property phase.
• MoveNextValidEventProperty then skips any dictionary entries flagged as message properties, yielding only true event properties.
• Reset() restores both_messagePropertiesIndex and_eventEnumerator from the dictionary’s current state, soKeyCollection/ValueCollection enumerations after a reset see the same sequence as an initial enumerator.

Given this,Count,Keys,Values, andforeach over the dictionary should all observe a coherent view of the combined properties.

Also applies to: 594-619, 626-630

tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1)

39-273:Benchmarks appropriately cover inline vs dictionary scenarios

The benchmark suite exercises exactly the scenarios this PR targets:

• Creation benchmarks for 1–4 properties show the allocation/promotion behavior.
• Lookup / not-found lookup / enumeration benches run repeated operations on a fresh dictionary per method, usingGetPropertyEnumerator() so the custom enumerator is also covered.
• The simpleif checks inside the loops are enough to prevent the JIT from optimizing the work away without distorting the microbenchmark too much.

Looks good as a basis for comparing the small-array representation against the always-dictionary baseline.

Reviewing files that changed from the base of the PR and betweenf8e3d6c and924795e.

• src/NLog/Internal/PropertiesDictionary.cs (17 hunks)
• src/NLog/LogEventInfo.cs (5 hunks)
• tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1 hunks)
• tests/NLog.UnitTests/LogMessageFormatterTests.cs (1 hunks)

• GitHub Check: task-list-completed

src/NLog/Internal/PropertiesDictionary.cs (1)

206-221:LGTM! Indexer setter enables array-backed mutations.

The new indexer setter correctly routes toAddOrUpdateMessageProperties, enabling in-place updates for the small-array optimization. TheallowUpdate: true parameter ensures existing properties can be overwritten without throwing.

tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1)

39-272:LGTM! Comprehensive benchmarks for array-backed optimization.

The benchmarks appropriately cover the key operations (create, lookup, not-found lookup, enumerate) across the 1–4 property range where the small-array optimization applies. The consistent patterns across all 16 methods make the performance comparisons reliable. These benchmarks align well with the PR objectives demonstrating allocation reduction and performance improvements for 1–3 properties.

tests/NLog.UnitTests/LogMessageFormatterTests.cs (1)

228-246:Unable to verify duplicate test method due to repository access issues.

The verification cannot be completed as the repository failed to clone. The original concern about potential duplicateMessageTemplatePositional test methods intests/NLog.UnitTests/LogMessageFormatterTests.cs requires direct file inspection to confirm or refute.

To complete this verification:

• Check if multipleMessageTemplatePositional method declarations exist in the file (search for method name and line numbers)
• If duplicates are found, confirm whether they have identical or different signatures
• Remove any duplicate methods to prevent compilation errors or test confusion

Reviewing files that changed from the base of the PR and between924795e and219f411.

• src/NLog/Internal/PropertiesDictionary.cs (17 hunks)
• src/NLog/LogEventInfo.cs (5 hunks)
• tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1 hunks)
• tests/NLog.UnitTests/LogMessageFormatterTests.cs (1 hunks)

• GitHub Check: task-list-completed

src/NLog/LogEventInfo.cs (1)

414-445:Array-based template-parameter plumbing looks consistent with new PropertiesDictionary semantics

The migration from IList toMessageTemplateParameter[] inTryCreatePropertiesInternal,CreatePropertiesInternal,HasMessageTemplateParameters,MessageTemplateParameters, and the immutability helpers appears internally consistent:

• TryCreatePropertiesInternal correctly:

  • Creates_properties from a non-emptytemplateParameters array.
  • Resets message properties on an existing dictionary viaResetMessageProperties(templateParameters, templateParameters.Length).
  • Defers toHasMessageTemplateParameters only whentemplateParameters is null, avoiding unnecessary work.

• CreatePropertiesInternal usesPropertiesDictionary(int initialCapacity) when there is no template array, andPropertiesDictionary(MessageTemplateParameter[] templateParameters) when there is, neatly aligning with the small-array vs. dictionary promotion inPropertiesDictionary.

• HasMessageTemplateParameters’ guard on_formattedMessage is null && _parameters?.Length > 0 prevents repeated template probing once formatting is done, and the use ofILogMessageFormatter.HasProperties(this) matches the intended “cheap probe, full parse later” pattern.

• MessageTemplateParameters now correctly depends on_properties?.MessageProperties returning the filteredMessageTemplateParameter[] (which, per the newPropertiesDictionary.MessageProperties implementation, excludes event-only small-array slots), and only falls back to reparsing_message +_parameters when no captured metadata exists.

• IsLogEventThreadAgnosticImmutable andHasImmutableProperties useproperties.MessageProperties.Length rather than a listCount, and the “fast path”properties.Count == _parameters?.Length && properties.Count == properties.MessageProperties.Length still correctly identifies the pure message-template case.

• ResetMessageTemplateParameters’ final conditionreturn _properties.MessageProperties.Length == 0; nicely leverages the new filtering logic onMessageProperties to decide when it is safe to re-parse templates.

I don’t see correctness or threading regressions introduced by these changes; they align with the new array-backedPropertiesDictionary behavior.

Also applies to: 447-459, 465-481, 700-719, 721-747, 832-845

src/NLog/Internal/PropertiesDictionary.cs (5)

52-86:Small-array backing and promotion thresholds are sound

UsingSmallArraySize = 3 with_messageProperties +_messagePropertiesCount, and only allocating_eventProperties wheninitialCapacity > SmallArraySize or on promotion, matches the PR objective of avoiding aDictionary for ≤3 properties:

• PropertiesDictionary(MessageTemplateParameter[]? messageParameters) andPropertiesDictionary(int initialCapacity) correctly steer between array and dictionary paths.
• IsEmpty andCount => _eventProperties?.Count ?? _messagePropertiesCount keep the public surface consistent regardless of backing store.

This is a clean, low-overhead way to introduce the small-array optimization without exposing it to callers.

Also applies to: 91-110, 196-201

---

112-140:Promotion to dictionary with capacity hint and cleanup of inline slots

GetEventProperties andInitializeEventPropertiesDictionary correctly:

• Promote from the small-array representation by seeding theDictionary<object, PropertyValue> from_messageProperties with an appropriate capacity (messagePropertiesCount ormessagePropertiesCount + 2 whenprepareForInsert).
• UseresetMessageProperties fromInsertMessagePropertiesIntoEmptyDictionary to distinguish:
  • “True” message-template properties (IsMessageProperty = true, capture type ≠Unknown).
  • Small-array event-only slots (allCaptureType.Unknown), which are migrated and then cleared via_messageProperties = null; _messagePropertiesCount = 0;.

ResetMessageProperties then builds on this by:

• Creating a fresh dictionary when needed (existing event properties, duplicates, or old message properties present).
• Removing prior template-derived entries viaRemoveOldMessageProperties.
• Re-inserting new message-template parameters with correctIsMessageProperty flags.

Overall, the promotion and reset logic looks correct and should avoid both capacity thrash and stale template entries.

Also applies to: 162-188

---

341-382:Lookup and small-array add/update paths behave correctly for both event and template scenarios

The new code inTryLookupMessagePropertyValue,AddOrUpdateMessageProperties, andSkipDictionaryAllocation looks solid:

• TryLookupMessagePropertyValue:
  • Early-outs when_messagePropertiesCount == 0.
  • Uses dictionary lookup for large message property sets (_messagePropertiesCount > 10), otherwise falls back to cheap linear scans with ordinal or case-insensitive comparisons.
• SkipDictionaryAllocation ensures:
  • Up to 3 string keys are stored in the_messageProperties array without overflowing (_messagePropertiesCount < _messageProperties.Length).
  • Any non-string key, or the 4th+ property, immediately promotes to the dictionary path.
• AddOrUpdateMessageProperties:
  • Reuses the array slot whenallowUpdate is true and the key already exists, otherwise throws on duplicate forAdd(...).
  • Correctly migrates existing small-array entries into a dictionary when the array is full, by callingGetEventProperties(true) and then adding/updating in the dictionary.

I don’t see anIndexOutOfRangeException path remaining here; the array-bound precondition is now enforced bySkipDictionaryAllocation.

Also applies to: 384-418

---

424-447:Uniqueness check and insertion deal well with duplicates and event-only arrays

VerifyUniqueMessageTemplateParametersFast andInsertMessagePropertiesIntoEmptyDictionary cover the tricky cases:

• VerifyUniqueMessageTemplateParametersFast:
  • Returnstrue for 0–1 parameters.
  • Forces dictionary usage for >10 parameters.
  • Performs an O(n²) duplicate scan only for small lists, which is acceptable given the threshold.
• InsertMessagePropertiesIntoEmptyDictionary:
  • UsesresetMessageProperties to detect arrays whereall entries areCaptureType.Unknown (event-only), marking those dictionary values as non-message properties (IsMessageProperty = false).
  • OnArgumentException fromDictionary.Add, only rewrites the name for clashes where the existing entry is a message property, usingGenerateUniquePropertyName and updating theMessageTemplateParameter in-place.

This keeps event-only usage distinct while preserving clear semantics for template-derived parameters, including duplicate names.

Also applies to: 452-485

---

501-529:Enumerator now aligns with Count and supports reset correctly

The updatedPropertyDictionaryEnumerator:

• Initializes_messagePropertiesIndex based on_dictionary._messagePropertiesCount and_eventEnumerator from_dictionary._eventProperties.
• Uses:
  • MoveNextValidMessageParameter to emit message-template entries first:
    • Iterates over_messagePropertiesCount.
    • For dictionary-backed cases, only yields entries whosePropertyValue.IsMessageProperty istrue, avoiding duplication.
  • MoveNextValidEventProperty to then enumerate only non-message entries from the dictionary (IsMessageProperty == false).
• Current,CurrentParameter, andCurrentProperty consistently index into_dictionary._messageProperties or_eventEnumerator depending on_messagePropertiesIndex.HasValue.
• Reset() now restores both_messagePropertiesIndex and_eventEnumerator to their initial states, so post-reset iteration covers the same items as the original enumeration.

GivenCount is_eventProperties?.Count ?? _messagePropertiesCount, the enumerator will yield exactlyCount elements in all configurations (small-array only, dictionary-only, or mixed). This fixes the earlier reset/enumerator issues without introducing new ones.

Also applies to: 531-547, 548-565, 568-581, 593-619, 625-629

tests/NLog.UnitTests/LogMessageFormatterTests.cs (1)

228-246:Positional-template test correctly exercises property override behavior

This test nicely validates the key invariant for positional templates:

• MessageTemplateParameters remains:
  • Valid and positional.
  • Aligned withlogEventInfo.Parameters ("Foo",123,Guid), even afterProperties["0"],["1"],["2"] are overridden.
• Properties reflects only the overridden values (HelloWorld1/2/3) with aCount of 3.

That matches the new design where message-template metadata stays intact in_messageProperties, while event properties (including overrides) flow throughPropertiesDictionary via the small-array/dictionary paths.

tests/NLog.Benchmarks/Internal/PropertiesDictionaryBenchmark.cs (1)

39-345:Benchmarks effectively cover small-array vs. dictionary behavior

The benchmark suite looks well-structured:

• Create*Property scenarios measure construction + initial inserts across 1–5 keys, naturally driving the small-array path for ≤3 and promotion for ≥4.
• Lookup*Property andNotFoundLookup*Property perform 1000 iterations against hot dictionaries, which is appropriate for timing steady-state lookups and miss patterns.
• Enumerate*Property usesGetPropertyEnumerator() andMoveNext() in a tight loop, which directly exercises the updatedPropertyDictionaryEnumerator over both small-array and dictionary-backed cases.

I don’t see correctness issues here; any micro-optimizations (e.g., removing theif (Current.Key is null) branch) would be purely cosmetic for your purposes.