Skip to content

Optimize DispatchProperties and ReceiveProperties storage#7843

Open
danielmarbach wants to merge 11 commits into
masterfrom
optimize_properties
Open

Optimize DispatchProperties and ReceiveProperties storage#7843
danielmarbach wants to merge 11 commits into
masterfrom
optimize_properties

Conversation

@danielmarbach

@danielmarbach danielmarbach commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Applies the same trick as shown in:

DispatchProperties and ReceiveProperties get allocated on every outgoing and incoming message. Both were backed by Dictionary<string, string>, so every instance paid for a full dictionary allocation regardless of whether it held zero entries or three.

This swaps the dictionary for inline storage. DispatchProperties now stores its three well-known properties (DoNotDeliverBefore, DelayDeliveryWith, DiscardIfNotReceivedBefore) in dedicated fields, with a small inline array for the custom properties that transports occasionally add. ReceiveProperties uses an inline array with an overflow dictionary that almost never needs to allocate.

The copy constructor has a fast path. When the source is already a DispatchProperties, it copies fields and inline slots directly instead of iterating key-value pairs.

Benchmarks on an M3 Max (BenchmarkDotNet, SimpleJob):

DispatchProperties:

  • Indexer set for well-known keys: ~18 ns to ~0.6 ns, 216 B to 0 B
  • ContainsKey for well-known keys: ~4 ns to ~0.3 ns
  • Copy construction: ~72 ns to ~9 ns, 488 B to 88 B
  • Typed property set: ~28 ns to ~8 ns, 256 B to 40 B
  • Construction with one well-known property: ~29 ns to ~13 ns, 256 B to 128 B
  • Construction with a custom property: ~19 ns to ~4 ns, 216 B to 88 B

ReceiveProperties:

  • Indexer get: ~2.6 ns to ~0.5 ns
  • ContainsKey and TryGetValue are essentially free when entries fit inline
  • Construction is a bit slower because we now copy entries instead of aliasing the source dictionary. Total system allocation is still better since the caller no longer allocates a Dictionary.

Two side effects. The acceptance test framework's DeepCopy utility used ValueType.GetHashCode() on boxed structs, which throws for [InlineArray] types. Switched to RuntimeHelpers.GetHashCode(), matching the BCL ReferenceEqualityComparer. And both types now guard null keys explicitly to match the Dictionary<string, string> contract.

The change is backward compatible at the source level. All five downstream transport packages (RabbitMQ, SQL Server, Azure Storage Queues, Amazon SQS, Azure Service Bus) were audited and use only the typed properties and the indexer.

All six persistence packages (DynamoDB, MongoDB, NHibernate, SQL, CosmosDB, Azure Table) were also audited. None of them serialize DispatchProperties directly. Each persists outbox transport operations through a storage DTO whose Options is a concrete Dictionary<string,string>, so the serializer only ever sees a plain Dictionary and the base-type change is invisible on the wire. The only source break found was a MongoDB test helper whose generic constraint referenced Dictionary<string,string>, relaxed to IDictionary<string,string> in Particular/NServiceBus.Storage.MongoDB#982.

Review notes. DispatchProperties keeps the original Dictionary<string,string> constructor (delegating to the IDictionary one) and now also implements IReadOnlyDictionary<string,string>, so passing it to a read-only dictionary parameter keeps compiling. The base-type change is binary-breaking by necessity, because the optimization only works by not inheriting from Dictionary. Binary compatibility isn't part of the release policy, which frames breaking changes as source-level, so this ships as a minor rather than a major.

@danielmarbach danielmarbach changed the title Optimize Dispatch and ReceiveProperties Optimize DispatchProperties and ReceiveProperties storage Jul 2, 2026
@danielmarbach danielmarbach added this to the vNext milestone Jul 2, 2026
@danielmarbach
danielmarbach marked this pull request as ready for review July 2, 2026 12:33
/// Creates a new instance of <see cref="DispatchProperties"/> an copies the values from the provided dictionary.
/// </summary>
public DispatchProperties(Dictionary<string, string> properties) : base(properties ?? [])
public DispatchProperties(IDictionary<string, string> properties)

@bording bording Jul 9, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changing this constructor makes this a binary breaking change, which is not something I'm comfortable with.

Do we have to change the signature of the ctor to get the benefit of this PR? If so, can we keep the original ctor and add this IDictionary one and maybe have the original one call this new one?

EDIT: The more I think about it, it's not clear to me that the change in base type is actually safe here, since Dictionary implements more interfaces than IDictionary.

This is feeling more and more like a "next major" sort of change.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right that the base-type change is binary-breaking, and it's unavoidable here. The whole reason this drops the per-message Dictionary allocation is that it stops inheriting from Dictionary, so we can't keep the base type without giving up the optimization.

I took your suggestions on board:

  • Kept the original DispatchProperties(Dictionary<string,string>) ctor; it delegates to the IDictionary one.
  • Added IReadOnlyDictionary<string,string>. IDictionary already covers the mutable generic surface, but IReadOnlyDictionary is the one realistic gap left, since passing a dictionary to a read-only parameter is a common pattern. The cost was small: Count, ContainsKey, TryGetValue and the read indexer are already satisfied by the existing members, so only Keys and Values needed explicit implementations. I skipped the non-generic IDictionary/ICollection and ISerializable, since I don't think they earn their keep for the sake of "compatibility".

On the "next major" part, I'd like to push back gently. As far as I can tell, binary compatibility isn't part of our contract. The release policy follows SemVer and frames breaking changes as source-level ("upgrading is likely to require code changes"), and nothing in the build enforces binary compatibility. So under our own policy, I don't think a binary break alone is the next major trigger. Also mind you this is a "low-level seam" that you almost only ever "use" as DispatchProperties and "just works" before and after this change.

To back the source-compat side, I audited all five transports and all six persistences. None of them cast to Dictionary or serialize DispatchProperties directly. They persist outbox operations through a storage DTO whose Options is a concrete Dictionary<string,string>, so the base-type change is invisible on the wire. The one real source break was a MongoDB test helper constrained to Dictionary<string,string>, which I relaxed to IDictionary<string,string> in
Particular/NServiceBus.Storage.MongoDB#982.

If you read the policy differently, or if there's a consumer surface I've missed, I'm happy to reconsider. The base-type change is the one thing we can't undo without losing the perf win, so it's worth getting the call right.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants