Avoid allocations due to ConcurrentStack usage

[Erarndt]

Fixes #

Context

The implementation of ConcurrentStack<T> creates a new wrapper node each time an item is pushed onto the stack

public void Push(T item)
{
    Node node = new Node(item);
    node.m_next = m_head;
    if (Interlocked.CompareExchange(ref m_head, node, node.m_next) != node.m_next)
    {
        PushCore(node, node);
    }
}

The creates an appreciable amount of allocations during build:

These allocations can almost entirely be eliminated by using a vanilla generic Stack<T> with a lock.

Changes Made

Testing

Notes

[Copilot]

Pull Request Overview

This PR replaces the thread‐safe ConcurrentStack<GenericExpressionNode> pools with a plain Stack<GenericExpressionNode> guarded by a lock to eliminate per‐push node allocations and reduce GC pressure.

• Swaps all ConcurrentStack<GenericExpressionNode> usage to Stack<GenericExpressionNode> in the cache structure.
• Adds a lock(expressionPool) around popping/parsing logic in EvaluateConditionCollectingConditionedProperties.
• Updates the ExpressionTreeForCurrentOptionsWithSize constructor and GetOrAdd signatures to use Stack<GenericExpressionNode>.

Comments suppressed due to low confidence (3)

src/Build/Evaluation/ConditionEvaluator.cs:251

• Add unit tests that simulate concurrent access to the new Stack<GenericExpressionNode> pools, ensuring that both retrieval and return of parsed expressions under lock correctly preserve pool integrity and reuse.

            lock (expressionPool)

src/Build/Evaluation/ConditionEvaluator.cs:251

• Only the pop-and-parse logic is wrapped by this lock, but the subsequent push back into the pool (still using Stack.Push) occurs outside it. Since Stack<T> is not thread-safe, wrap both pop and push operations in the same lock to avoid race conditions.

            lock (expressionPool)

src/Build/Evaluation/ConditionEvaluator.cs:255

• Declaring parsedExpression inside the lock block can limit its scope if you later need to use it outside. Consider moving the declaration immediately before the lock so its value is accessible throughout the remainder of the method.

                GenericExpressionNode parsedExpression;

[AR-May]

Looks good to me. I ran exp insertion, let's see if it passes.

[JanProvaznik]

looks correct in the reasoning and code, let's see insertion

[AR-May]

exp insertion is good: https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/663329