perf: Add caching to AttributeHelpers.GetCustomAttributesIncludingBaseInterfaces

[thomhurst]

Summary

• Added a ConcurrentDictionary<(Type, Type), object[]> cache to store attribute lookup results
• Cache key is a tuple of (Type, AttributeType) for efficient lookups
• Eliminates redundant reflection calls for repeated attribute queries on the same type

Fixes #1532

Test plan

• Verify the project builds successfully
• Existing unit tests should pass unchanged (behavior is identical, only performance improves)
• Verify caching behavior in scenarios with repeated attribute lookups

🤖 Generated with Claude Code

[Copilot]

Pull request overview

This PR adds caching to AttributeHelpers.GetCustomAttributesIncludingBaseInterfaces to improve performance by eliminating redundant reflection calls for repeated attribute queries on the same type.

Key Changes:

• Introduced a ConcurrentDictionary cache with composite keys of (Type, AttributeType)
• Refactored the method from using yield return to materializing results into an array for caching
• Created a new internal helper method GetAttributesInternal to separate attribute collection from caching logic

[Copilot]

Using the indexer assignment Cache[key] = attributes can lead to redundant computation if multiple threads race to populate the same cache entry. Consider using GetOrAdd to ensure the computation happens only once per key, or use TryAdd to avoid overwriting an existing entry that another thread may have added.

Suggested change

	if (Cache.TryGetValue(key, out var cachedResult))
	{
	return cachedResult.Cast<T>();
	}
	var attributes = GetAttributesInternal<T>(type);
	Cache[key] = attributes;
	var attributes = Cache.GetOrAdd(key, _ => GetAttributesInternal<T>(type));

[Copilot]

The cache grows unbounded and will never evict entries, which could lead to memory issues in long-running applications that work with many different types. Consider implementing a cache eviction policy (e.g., LRU, size limit) or documenting this limitation if the unbounded growth is acceptable for the expected usage patterns.

[thomhurst]

Summary

Adds caching to attribute lookups using ConcurrentDictionary to improve performance during module dependency resolution.

Critical Issues

None found ✅

Suggestions

Consider memory lifecycle: The cache is static and lives for the entire application lifetime. While this is likely fine for typical pipeline scenarios (single execution per process), consider if there are any long-running scenarios where modules could be dynamically loaded/unloaded.

Minor optimization: The current implementation creates a List and converts to array. You could use ToArray() directly on the LINQ expressions to avoid the intermediate list. However, the current implementation is clearer and the performance difference is negligible.

Verdict

✅ APPROVE - No critical issues

This follows the same performance optimization pattern as PR #1726. The caching is appropriate since attributes are compile-time metadata that doesn't change at runtime. The use of ConcurrentDictionary ensures thread-safety for parallel module resolution.