Skip to content

Remove Logger and RoslynCompilerEventSource - #1057

Merged
pharring merged 1 commit into
dotnet:masterfrom
pharring:RemoveLogger
Mar 6, 2015
Merged

Remove Logger and RoslynCompilerEventSource#1057
pharring merged 1 commit into
dotnet:masterfrom
pharring:RemoveLogger

Conversation

@pharring

@pharring pharring commented Mar 5, 2015

Copy link
Copy Markdown
Contributor

Remove the Logger and ETW event source from the compiler layer. Addresses #1007

@pharring

pharring commented Mar 5, 2015

Copy link
Copy Markdown
Contributor Author

@agocke @heejaechang Please review. This should be simple, it's just deleting code :-)

@agocke

agocke commented Mar 5, 2015

Copy link
Copy Markdown
Member

LGTM, thanks for taking this one!

@agocke

agocke commented Mar 5, 2015

Copy link
Copy Markdown
Member

BTW, this change is way easier to review if you add &w=1 to your URL -- it'll hide the whitespace changes.

@heejaechang

Copy link
Copy Markdown
Contributor

adding @shyamnamboodiripad

@heejaechang

Copy link
Copy Markdown
Contributor

looks good to me.

...

so, I guess compiler doesnt need etw for perf investigation at all? I find it quite useful when trying to improve perf of feature.

I used it to improve FAR, HR, formatting engine, tagger and more. basically it let me see sequence of all events and time each pipe line took.

@VSadov

VSadov commented Mar 5, 2015

Copy link
Copy Markdown
Member

LGTM

pharring added a commit that referenced this pull request Mar 6, 2015
Remove Logger and RoslynCompilerEventSource
@pharring
pharring merged commit ae1205f into dotnet:master Mar 6, 2015
@pharring
pharring deleted the RemoveLogger branch March 6, 2015 00:44
@shyamnamboodiripad

Copy link
Copy Markdown
Contributor

LGTM. Its sad that we had to remove the logger code entirely - but I guess we can always bring this back if we need to.

@agocke

agocke commented Mar 6, 2015

Copy link
Copy Markdown
Member

@shyamnamboodiripad @heejaechang This is why we have version control :) All this code is a revert away (if we really want it).

@shyamnamboodiripad

Copy link
Copy Markdown
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants