Add custom comparer to use incremental generator caching for LoggerMessageGenerator and JsonSourceGenerator

[3schwartz]

Related to issue #74557, this is a proposed solution how to add caching to LoggerMessageGenerator and JsonSourceGenerator.

[ghost]

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

Issue Details

---

Related to issue #74557, this is a proposed solution how to add caching to LoggerMessageGenerator and JsonSourceGenerator.

Author:	3schwartz
Assignees:	-
Labels:	area-Extensions-Logging
Milestone:	-

[teo-tsirpanis]

Ignoring the Compilation when comparing the tuples is not correct according to #64579 (comment).

[eerhardt]

@CyrusNajmabadi @chsienki - are changes still needed here with the latest changes using the new Roslyn Attribute API?

[tarekgh]

CC @dotnet/area-system-text-json as there is a change in the System.Text.Json too.

[3schwartz]

@CyrusNajmabadi @chsienki - are changes still needed here with the latest changes using the new Roslyn Attribute API?

Hi @eerhardt
Could you help share which changes you are talking about? Like a link to some issue. I would very much like to read up upon it.

[eerhardt]

Could you help share which changes you are talking about? Like a link to some issue. I would very much like to read up upon it.

No problem. Check out:

dotnet/roslyn#54725
#70754
#70911

[3schwartz]

Ignoring the Compilation when comparing the tuples is not correct according to #64579 (comment).

Thanks for your reply.

Looking at the PR you linked, it seems like you moved a lot of the logic within the ´GetSemanticTargetForGeneration´ instead of having this executed after RegisterSourceOutput is called.

If you think it could help and changes would be accepted, I could give it a try with the above two generators and move the logic executed after RegisterSourceOutput to the predicate, such that optimal caching is used.

Please let me know if help is appreciated :-)

I will of course wait and see what is answered to the question regarding if a new Roslyn Attribute API has fixed the issue.

[eiriktsarpalis]

See #68353 (comment):

It also employs a custom equality comparer that ignores the Compilation component

Never do this. This action instructs the command line compiler that it is allowed to produce inaccurate outputs in an actual .dll or .exe, and will allow a production machine to publish/sign/ship that invalid output.

cc @sharwell

EDIT I missed that this was already mentioned in the comments above.

[3schwartz]

Could you help share which changes you are talking about? Like a link to some issue. I would very much like to read up upon it.

No problem. Check out:

dotnet/roslyn#54725 #70754 #70911

Hi @eerhardt

After reading through the links you send I understand it as the API ForAttributeWithMetadataName now does the correct filtering in the predicate and avoid people doing the logic after register the incremental values (hope it is correct understood).

It also seems like some are working on updating the generators. I will close the PR and issue then - thanks clarifying I’m looking forward to the API is out 😊