Skip to content

Avoid some allocations in MiniYaml.Merge.#21558

Merged
abcdefg30 merged 1 commit into
OpenRA:bleedfrom
RoosterDragon:yaml-alloc
Sep 23, 2024
Merged

Avoid some allocations in MiniYaml.Merge.#21558
abcdefg30 merged 1 commit into
OpenRA:bleedfrom
RoosterDragon:yaml-alloc

Conversation

@RoosterDragon
Copy link
Copy Markdown
Member

@RoosterDragon RoosterDragon commented Aug 22, 2024

During the merge operation, it is quite common to be dealing with a node that has no child nodes. When there are no such nodes, we can return early from some functions to avoid allocating new collections that will not be used.

In the MergePartial operation, reuse a dictionary as scratch space when checking for conflicts. We introduce a IntoDictionaryWithConflictLog helper to allow this. This avoids allocating a new dictionary for the conflict log that gets thrown away at each check.

During the merge operation, it is quite common to be dealing with a node that has no child nodes. When there are no such nodes, we can return early from some functions to avoid allocating new collections that will not be used.

In the MergePartial operation, reuse a dictionary as scratch space when checking for conflicts. We introduce a IntoDictionaryWithConflictLog helper to allow this. This avoids allocating a new dictionary for the conflict log that gets thrown away at each check.
Comment thread OpenRA.Game/MiniYaml.cs
Copy link
Copy Markdown
Member

@pchote pchote left a comment

Choose a reason for hiding this comment

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

LGTM, and appears to behave correctly in both normal and failure cases.

@abcdefg30 abcdefg30 merged commit b4882a8 into OpenRA:bleed Sep 23, 2024
@abcdefg30
Copy link
Copy Markdown
Member

Changelog

@RoosterDragon RoosterDragon deleted the yaml-alloc branch September 23, 2024 18:09
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.

4 participants