Skip to content

Conversation

@snakefoot
Copy link

@snakefoot snakefoot commented Nov 30, 2025

Resolves #122046 (Removed "expensive" Count-check, since also exists in LogValuesFormatter)

Method Mean Error StdDev Gen0 Allocated
FormattedLogValues_Lookup (Before) 5.429 ns 0.0971 ns 0.0908 ns - -
FormattedLogValues_Lookup (After) 4.295 ns 0.0159 ns 0.0149 ns - -

Benchmark code:

    public class FormattedLogValuesTest
    {
        FormattedLogValues _test = new FormattedLogValues("{0} {1} {2}", new object[] { "Hello", "Cruel", "World" });

        [Benchmark]
        public void FormattedLogValues_Lookup()
        {
            var parameterCount = _test.Count;
            for (int i = 0; i < parameterCount; ++i)
            {
                var parameter = _test[i];
                if (parameter.Key is null)
                    return;
            }
        }
    }

Copilot AI review requested due to automatic review settings November 30, 2025 12:50
@github-actions github-actions bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Nov 30, 2025
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Nov 30, 2025
@snakefoot snakefoot changed the title FormattedLogValues - Small optimization for index operatoe FormattedLogValues - Small optimization for index operator Nov 30, 2025
@snakefoot snakefoot force-pushed the FormattedLogValuesIndexOperator branch from 5bec10c to 437108d Compare November 30, 2025 12:51
Copilot finished reviewing on behalf of snakefoot November 30, 2025 12:53
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR optimizes the indexer operator in FormattedLogValues by removing a redundant check for the last element (original format). Previously, the indexer explicitly checked if index == Count - 1 to return the original format, but this check was redundant since LogValuesFormatter.GetValue() already handles this case. The optimization reduces execution time by ~20% (from 5.4ns to 4.3ns) by delegating all logic to GetValue when a formatter exists, eliminating the duplicate check.

Key changes:

  • Restructured indexer logic to check for null formatter/values upfront and delegate to GetValue otherwise
  • Removed redundant index == Count - 1 check since GetValue handles the last index internally

@snakefoot snakefoot force-pushed the FormattedLogValuesIndexOperator branch from dc672d0 to b83ddbe Compare November 30, 2025 13:03
@jkotas jkotas added area-Extensions-Logging and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Nov 30, 2025
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-extensions-logging
See info in area-owners.md if you want to be subscribed.

Copy link
Member

@tarekgh tarekgh left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @snakefoot!

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

Labels

area-Extensions-Logging community-contribution Indicates that the PR has been added by a community member

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Small FormattedLogValues Parameter Lookup Optimization

3 participants