Eliminate XmlChildNodes allocations in GetXmlNodeInnerContents

[nareshjo]

🤖 AI-Generated Pull Request 🤖

This pull request was generated by the VS Perf Rel AI Agent. Please review this AI-generated PR with extra care! For more information, visit our wiki. Please share feedback with TIP Insights

---

• Issue:
  Utilities.GetXmlNodeInnerContents accesses node.ChildNodes twice to check Count == 1.
  In .NET Framework, XmlNode.ChildNodes calls new XmlChildNodes(this) on every access — it never caches — so each call produces 2 throwaway heap allocations.
  This method is called for every property and metadata element during evaluation, making it a significant source of GC pressure during solution load.

  Allocation sites (TypeAllocated!System.Xml.XmlChildNodes, ~86% of sampled allocations):

  GetXmlNodeInnerContents
   → XmlNode.get_ChildNodes        ← allocates new XmlChildNodes(this) every call
     → XmlChildNodes.get_Count     ← iterates all children, result used once
       → compared to == 1, wrapper discarded
  

  Callers on hot path as per Perfwatson traces:

  EvaluatePropertyElement → GetXmlNodeInnerContents        (~21%)
  ProcessMetadataElements → GetXmlNodeInnerContents      (~25%)
  DecorateItemsWithMetadata → GetXmlNodeInnerContents    (~40%)
  

• Issue type: Avoid allocating wrapper objects when allocation-free alternatives exist on the same API surface.

• Proposed fix: Replace node.ChildNodes.Count == 1 with node.FirstChild.NextSibling == null — a simple pointer check that answers the same question ("is there exactly one child?") with zero allocations. Cache FirstChild in a local and split the compound if into clear separate checks. This is a direct semantic equivalence (FirstChild == null ↔ !HasChildNodes, NextSibling == null ↔ Count == 1), internal-only, and produces identical results for all edge cases.

Best practices wiki
See related failure in PRISM
ADO work item

[Copilot]

Pull request overview

Reduces GC pressure in Utilities.GetXmlNodeInnerContents by avoiding repeated XmlNode.ChildNodes accesses (which allocate XmlChildNodes wrappers on .NET Framework) and using allocation-free FirstChild/NextSibling checks instead.

Changes:

• Replaces node.ChildNodes.Count == 1 checks with firstChild.NextSibling == null to avoid per-call XmlChildNodes allocations.
• Caches node.FirstChild and simplifies the “no children / single whitespace child / single text or CDATA child” fast paths.

[JanProvaznik]

running experimental insertion to see impact on perf metrics

[JanProvaznik]

https://devdiv.visualstudio.com/DevDiv/_git/VS/pullrequest/727231 impact visible on speedometer

thanks