Skip to content

Avoid unused NetEventSource code #73868

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.

Sign up for GitHub

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

Jump to bottom
Merged
MihaZupan merged 5 commits into from
Aug 15, 2022
Merged

Avoid unused NetEventSource code #73868

MihaZupan merged 5 commits into from
Aug 15, 2022

Conversation

@MihaZupan
Copy link
Member

@MihaZupan MihaZupan commented Aug 12, 2022 •
edited
Loading

Fixes #54859

  • Removed a copy of NetEventSource.Common.cs from the aspnetcore shared folder - completely unused
  • Split of Associate and DumpBuffer methods into separate partial files that are optionally included by projects that actually use them
  • Removed as many constants and code from the NetEventSource.Common where possible (e.g. only used by a single project)
  • Moved some Windows-specific logging code out of the shared class
  • Removed a bunch of redundant IsEnabled checks

Release assembly sizes:

Windows (-10 KB):

Assembly Before After Diff
System.Net.Http.dll 632320 631808 -512
System.Net.Http.WinHttpHandler.dll 136704 130560 -6144
System.Net.HttpListener.dll 269312 268288 -1024
System.Net.NetworkInformation.dll 76288 74752 -1536
System.Net.Security.dll 248832 247808 -1024

Linux: (-3.5 KB)

Assembly Before After Diff
System.Net.Http.dll 622080 621568 -512
System.Net.HttpListener.dll 121856 121344 -512
System.Net.NetworkInformation.dll 82432 80896 -1536
System.Net.Security.dll 301056 300544 -512
System.Net.Sockets.dll 218624 218112 -512

@ghost ghost assigned MihaZupan Aug 12, 2022
@ghost
Copy link

ghost commented Aug 12, 2022

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

Issue Details

Fixes #54859

  • Removed a copy of NetEventSource.Common.cs from the aspnetcore shared folder - completely unused
  • Split of Associate and DumpBuffer methods into separate partial files that are optionally included by projects that actually use them
  • Removed as many constants and code from the NetEventSource.Common where possible (e.g. only used by a single project)
  • Moved some Windows-specific logging code out of the shared class
  • Removed a bunch of redundant IsEnabled checks

Release assembly sizes:

Windows (-10 KB):

Assembly Before After Diff
System.Net.Http.dll 632320 631808 -512
System.Net.Http.WinHttpHandler.dll 136704 130560 -6144
System.Net.HttpListener.dll 269312 268288 -1024
System.Net.NetworkInformation.dll 76288 74752 -1536
System.Net.Security.dll 248832 247808 -1024

Linux:
ToDo

Author: MihaZupan
Assignees: -
Labels:

area-System.Net
Milestone: -

@MihaZupan
Copy link
Member Author

MihaZupan commented Aug 12, 2022

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 12, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@MihaZupan MihaZupan force-pushed the eventsource-size-cleanup branch from c14831e to d38203f Compare August 12, 2022 22:54
@MihaZupan MihaZupan marked this pull request as ready for review August 14, 2022 18:34
@CarnaViire
Copy link
Member

CarnaViire commented Aug 15, 2022

Will you update the PR soon @MihaZupan ? 😄

@MihaZupan
Copy link
Member Author

MihaZupan commented Aug 15, 2022 •
edited
Loading

Yes, just waiting for VS to update :)

Soon™️

CarnaViire
CarnaViire approved these changes Aug 15, 2022
View reviewed changes
Copy link
Member

@CarnaViire CarnaViire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@MihaZupan
Copy link
Member Author

MihaZupan commented Aug 15, 2022

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 15, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@MihaZupan MihaZupan closed this Aug 15, 2022
@MihaZupan MihaZupan reopened this Aug 15, 2022
@MihaZupan
Copy link
Member Author

MihaZupan commented Aug 15, 2022 •
edited
Loading

Restarting CI after #73962

Regular CI was already clean.

I see no networking outerloop failures now.

@MihaZupan
Copy link
Member Author

MihaZupan commented Aug 15, 2022

/azp run runtime-libraries-coreclr outerloop

@azure-pipelines
Copy link

azure-pipelines bot commented Aug 15, 2022

Azure Pipelines successfully started running 1 pipeline(s).

@MihaZupan MihaZupan added this to the 7.0.0 milestone Aug 15, 2022
@MihaZupan MihaZupan merged commit 13733cd into dotnet:main Aug 15, 2022
@LakshanF LakshanF mentioned this pull request Aug 22, 2022
Open
@ghost ghost locked as resolved and limited conversation to collaborators Sep 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@CarnaViire CarnaViire CarnaViire approved these changes

Assignees

@MihaZupan MihaZupan

Labels

area-System.Net

Projects

None yet

Milestone

7.0.0

Development

Successfully merging this pull request may close these issues.

Custom EventSources in shared library seem to have unused code

2 participants

@MihaZupan @CarnaViire