Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve EventSource trimming #56154

Open
LakshanF opened this issue Jul 22, 2021 · 3 comments
Open

Improve EventSource trimming #56154

LakshanF opened this issue Jul 22, 2021 · 3 comments
Labels
area-System.Diagnostics.Tracing enhancement Product code improvement that does NOT require public API changes/additions
Milestone

Comments

@LakshanF
Copy link
Member

LakshanF commented Jul 22, 2021

Using EventSource to log data to a listener is fairly advanced (this blurb from EventSource describes the high level details). Even though .NET runtime take a lot of effort to hide the implementation details from developers who want to write their own EventSource, it can be still quite challenging to do so. A practical challenge that a user can run into can be seen at #27635.

Creating an EventSource and using it in a trimmed applications make this situation almost untenable currently. Issue #32863 discusses some of the challenges. The trimmer has to reason about the EventSource in libraries as well since there are increasingly proliferating number of EventSources in our runtime libraries.

In .NET6, there was an effort to address trimming challenges with enhancements to the trimmer and changes to the EventSource code. The details below captures the current status with regards the unique characteristics of EventSource in trim mode;

  1. EventSource is tagged with the attribute, DynamicallyAccessedMembersAttribute, at the class level. This was done primarily to address generating a manifest for a derived EventSource type.
    - This means that the trimmer will preserve members of the full type inheritance graph for EventSource. Specifically, this can lead to situation like, Custom EventSources in shared library seem to have unused code #54859, where the current expectation from runtime library developers that the trimmer will get rid of unused methods cannot be honored.
  2. EventSource has some event methods that are marked with RequiresUnreferencedCodeAttribute with parameters that could take non-primitive types. This was primarily to address the inability for the trimmer to handle complex serialization (there is a type recursion involving code here)
    - Any library EventSource that uses these methods need to be managed usually by adding UnconditionalSuppressMessageAttribute attribute after checking that the specific EventSource does not pass any non-primitive types to the parent method (or take steps to preserve the complex type).
  3. The above 2 can interact in complex ways and the trimmer needs to do the right thing
    - EventSources that do not use the complex parent methods to send event data should not get any warnings
    - EventSources that do use the complex parent need to see the warning with actionable details
  4. In spite of the work above, EventSource and its derived types continue to be treated in a special way in the trimmer in library mode trimming. In .NET 6.0, we moved the special handling logic to be only applicable in library mode via a flag (named DisableEventSourceSpecialHandling that is set to false in library mode). Specifically, preserve all static fields in nested types named "Keywords", "Tasks", "Opcodes" in EventSource derived types. Also preserve public instance methods that has the EventDataAttribute attribute.

It is not reasonable to expect developers to understand the intricacies of trimming when they write event sources. We should make trimming EventSource seamless. Some options we should consider include

  1. (Suggestion from @vitek-karas) Refactor EventSource code such that contract definition and implementation details are separate.
  2. Make progress on source generators for EventSources
  3. Encourage runtime library EventSource tests to run in trim mode
@LakshanF LakshanF added this to the 7.0.0 milestone Jul 22, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added area-System.Diagnostics.Tracing untriaged New issue has not been triaged by the area owner labels Jul 22, 2021
@ghost
Copy link

ghost commented Jul 22, 2021

Tagging subscribers to this area: @tarekgh, @tommcdon, @pjanotti
See info in area-owners.md if you want to be subscribed.

Issue Details

Using EventSource to log data to a listener is fairly advanced (this blurb from EventSource describes the high level details). Even though .NET runtime take a lot of effort to hide the implementation details from developers who want to write their own EventSource, it can be still quite challenging to do so. A practical challenge that a user can run into can be see here.

Creating an EventSource and using it in a trimmed applications make this situation almost untenable currently. This issue discusses some of the challenges. The trimmer has to reason about the EventSource in libraries as well since there are increasingly proliferating number of EventSources in our runtime libraries.

In .NET6, there was an effort to address trimming challenges with enhancements to the trimmer and changes to the EventSource code. The details below captures the current status with regards the unique characteristics of EventSource in trim mode;

  1. EventSource is tagged with the attribute, DynamicallyAccessedMembersAttribute, at the class level. This was done primarily to address generating a manifest for a derived EventSource type.
    - This means that the trimmer will preserve members of the full type inheritance graph for EventSource. Specifically, this can lead to situation like, EventSource contains code which ILLink can't analyze #32863, where the current expectation from runtime library developers that the trimmer will get rid of unused methods cannot be honored.
  2. EventSource has some event methods that are marked with RequiresUnreferencedCodeAttribute with parameters that could take non-primitive types. This was primarily to address the inability for the trimmer to handle complex serialization (there is a type recursion involving code here)
    - Any library EventSource that uses these methods need to be managed usually by adding UnconditionalSuppressMessageAttribute attribute after checking that the specific EventSource does not pass any non-primitive types to the parent method (or take steps to preserve the complex type).
  3. The above 2 can interact in complex ways and the trimmer needs to do the right thing
    - EventSources that do not use the complex parent methods to send event data should not get any warnings
    - EventSources that do use the complex parent need to see the warning with actionable details
  4. In spite of the work above, EventSource and its derived types continue to be treated in a special way in the trimmer in library mode trimming. In .NET 6.0, we moved the special handling logic to be only applicable in library mode via a flag (named DisableEventSourceSpecialHandling that is set to false in library mode). Specifically, preserve all static fields in nested types named "Keywords", "Tasks", "Opcodes" in EventSource derived types. Also preserve public instance methods that has the EventDataAttribute attribute.

It is not reasonable to expect developers to understand the intricacies of trimming when they write event sources. We should make trimming EventSource seamless. Some options we should consider include

  1. Encourage runtime library EventSource tests to run in trim mode
  2. Make progress on source generators for EventSources
Author: LakshanF
Assignees: -
Labels:

area-System.Diagnostics.Tracing, untriaged

Milestone: 7.0.0

@LakshanF
Copy link
Member Author

@jeffschwMSFT jeffschwMSFT removed the untriaged New issue has not been triaged by the area owner label Jul 22, 2021
@jkotas
Copy link
Member

jkotas commented Jul 22, 2021

Event source is an example of reflection-based serializer. Every reflection-based serializer faces very similar set of problems with trimming. I believe source generators are the only sustainable way to solve them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Diagnostics.Tracing enhancement Product code improvement that does NOT require public API changes/additions
Projects
None yet
Development

No branches or pull requests

4 participants