Cache reflection lookups in DynamicDataOperations

[Evangelink]

The Get*ConsideringInheritance methods (for fields, properties, and methods) walk the full type hierarchy via reflection on every call. This adds a ConcurrentDictionary cache per member kind, keyed by a (Type, memberName) struct, so the hierarchy walk happens only once per unique pair.

Uses a custom TypeMemberKey struct instead of ValueTuple for net462 compatibility.

The gain is most of the time neglictible but for some code bases where the same datasource is reused many times (e.g. MTP and MSTest acceptance tests) there is some gain.

I think it's not hurting readability and maintainance so it's probably worth it to keep it but I am fine if we prefer to close.

[Copilot]

Pull request overview

This PR improves DynamicData reflection performance in MSTest’s DynamicDataOperations by caching inheritance-walking member lookups (fields/properties/methods) so repeated calls don’t traverse the full type hierarchy every time.

Changes:

• Added per-member-kind ConcurrentDictionary caches keyed by (Type, memberName) (via a custom TypeMemberKey struct).
• Refactored the existing hierarchy-walk implementations into Lookup*InHierarchy helpers and routed Get*ConsideringInheritance through the caches.

---

You can also share your feedback on Copilot code review. Take the survey.

[Youssef1313]

• Have we seen real perf issues here that warrants adding a cache?
• I feel like walking the inheritance hierarchy is already not the right approach. Instead, we should avoid BindingFlags.DeclaredOnly and let the .NET runtime do its thing. The .NET reflection implementation has some caching already, so if we are able to avoid walking the hierarchy ourselves and let the caching behavior of reflection do its job, I think that's the best.

[Evangelink]

@Youssef1313 Great suggestion — replaced the manual hierarchy walk + caching with a single reflection call using BindingFlags.FlattenHierarchy instead of DeclaredOnly. This removes the ConcurrentDictionary caches, the TypeMemberKey struct, and the Lookup*InHierarchy helpers — net result is -49 lines, +4 lines. All 775 tests pass.

[Youssef1313]

LGTM, with two small comments.

[Copilot]

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated 2 comments.

---

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

This change switches from a most-derived-first, per-type lookup (via DeclaredOnly + manual BaseType walk) to a single reflection lookup using FlattenHierarchy. That is not behavior-equivalent: (1) BindingFlags.FlattenHierarchy will not return private static members declared on base types, whereas the previous implementation would find them; and (2) the single-call lookup can surface multiple matches across the inheritance chain and throw AmbiguousMatchException in cases where the previous implementation would have returned the derived member. If the intent is purely perf, consider keeping the previous resolution semantics (derived-first, include non-public base members) and introducing caching as described in the PR, rather than changing the BindingFlags behavior.

[Copilot]

The PR title/description mention adding ConcurrentDictionary caches (and a TypeMemberKey struct for net462), but this diff instead removes the custom hierarchy-walk helpers and performs direct reflection lookups with FlattenHierarchy—no caching is introduced. Please update the PR description/title to reflect the actual approach, or include the intended caching changes.

[Youssef1313]

I'm fine with the behavior change. Let's ensure that the analyzer that validates dynamic data handles it correctly though.

[Youssef1313]

In DynamicDataShouldBeValidAnalyzer, we have TryGetMemberCore local function. We can possibly add bool disallowPrivate there, and we pass false in the first iteration, and true on subsequent iterations.