Use Type.GetProperty/GetField instead of GetMember.

[eiriktsarpalis]

Following up from the discussion of dotnet/aspnetcore#56139. This PR replaces Type.GetMember for MemberInfo resolution with Type.GetField/GetProperty.

[MichalStrehovsky]

rt-sz doesn't report any improvement from this (MichalStrehovsky/rt-sz#30), but also it doesn't test with the TodosApi app so I don't know if it would help there. rt-sz saw a small regression with the PR that added this: MichalStrehovsky/rt-sz#28 but the regression was much smaller than what we saw with todosapi so I don't know :/

[MichalStrehovsky]

rt-sz doesn't report any improvement from this (MichalStrehovsky/rt-sz#30), but also it doesn't test with the TodosApi app so I don't know if it would help there. rt-sz saw a small regression with the PR that added this: MichalStrehovsky/rt-sz#28 but the regression was much smaller than what we saw with todosapi so I don't know :/

Ah, rt-sz wouldn't really capture improvements in changes like this. It doesn't pick up the new source generator. So a zero diff result is expected. We don't have a good way to measure impact of this in automation.

[eiriktsarpalis]

I wouldn't expect it to create a lot of impact size-wise. The main issue I would think is finding a good way to make the AttributeProviderFactory delegate trimmable if it isn't being invoked. Any suggestions?

[MichalStrehovsky]

I wouldn't expect it to create a lot of impact size-wise. The main issue I would think is finding a good way to make the AttributeProviderFactory delegate trimmable if it isn't being invoked. Any suggestions?

Is the public surface are immutable (i.e. we cannot unseal JsonPropertyInfoValues<T> and change how AttributeProviderFactory property is implemented by making it virtual or something like that)?

[eiriktsarpalis]

You mean have the source generator override a virtual method? Would that make it more amenable to trimming?

[MichalStrehovsky]

We're quite a bit limited that the typeof(Foo).GetProperty("Bar") needs to use literals everywhere because trimming needs to see this sequence. At this point I'm mostly thinking whether we can at least get rid of all the lambdas with something like:

class Provider
{
    private PropertyInfo _pi;
    public Provider(PropertyInfo pi) =>_pi = pi;
    public ICustomAttributeProvider Get() => _pi;
}

then do AttributeProviderFactory = new Provider(typeof(Foo).GetProperty("Bar")).Get;

Or even make a custom JsonPropertyInfoValues<T> descendant that overrides AttributeProviderFactory with this impl. I don't know if there's a chance to trim the typeof(Foo).GetProperty("Bar") part. At least I don't quickly see anything.

[eiriktsarpalis]

The lambda is there so that attribute provider resolution can be done lazily. By making it eager we'd end up penalizing startup in all uses of the source generator.

[eiriktsarpalis]

@MichalStrehovsky I'm thinking we should merge this change regardless, given that it has first-class support from the linker.

[MichalStrehovsky]

This is an improvement from trimming perspective because we no longer root all members on the type (GetMembers is not intrinsically recognized by trimming and will root all members on the typeof). GetProperty/GetField will only root that single field/property.

[eiriktsarpalis]

/ba-g unrelated infra failure