Skip to content

Remove redundant list#4448

Merged
dscpinheiro merged 3 commits into
aws:developmentfrom
martincostello:remove-redundant-allocation
Jun 24, 2026
Merged

Remove redundant list#4448
dscpinheiro merged 3 commits into
aws:developmentfrom
martincostello:remove-redundant-allocation

Conversation

@martincostello

Copy link
Copy Markdown
Contributor

Description

Remove allocation for a list that is never used.

Motivation and Context

Noticed while looking at fix for #4445.

Testing

None - trivial change.

Dry-runs

  • DotNet Dry-run ID:
    • Pending
    • Completed successfully
    • Failed
  • PowerShell Dry-run ID:
    • Pending
    • Completed successfully
    • Failed

Breaking Changes Assessment

N/A

Screenshots (if appropriate)

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist

  • My code follows the code style of this project
  • My change requires a change to the documentation
  • I have updated the documentation accordingly
  • I have read the README document
  • I have added tests to cover my changes
  • All new and existing tests passed

License

  • I confirm that this pull request can be released under the Apache 2 license

Remove allocation for a list that is never used.
Copilot AI review requested due to automatic review settings June 23, 2026 08:50
@martincostello martincostello requested review from a team as code owners June 23, 2026 08:50
@martincostello martincostello requested review from AlexDaines and dscpinheiro and removed request for a team June 23, 2026 08:50
@martincostello

martincostello commented Jun 23, 2026

Copy link
Copy Markdown
Contributor Author

Not changed in this PR, but depending on how hot this code path is, it could potentially benefit from the double-lock pattern to avoid acquiring the lock if the cache has a value already:

public static RegionEndpoint GetRegionEndpoint(bool includeInstanceMetadata)
{
lock(_lock)
{
if (cachedRegion != null)
return cachedRegion.Region;

The unconditional lock might explain the performance regression from #4445.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Removes an unnecessary allocation in the Core region fallback logic to reduce allocations during region resolution.

Changes:

  • Removed an unused List<Exception> allocation in FallbackRegionFactory.GetRegionEndpoint(...).
  • Simplified the exception handling path in the region generator loop.

Comment thread sdk/src/Core/Amazon.Runtime/AWSRegion.cs Outdated
Comment thread sdk/src/Core/Amazon.Runtime/AWSRegion.cs Outdated
@dscpinheiro

Copy link
Copy Markdown
Contributor

Started an internal build (DRY_RUN-15ce87e0-644e-413e-ba2e-715c79bb81f6). If it passes we'll merge the PR and it'll be included in the next SDK release.

@dscpinheiro dscpinheiro merged commit d78b0ec into aws:development Jun 24, 2026
2 of 3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants