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

Perf improvement #890

Merged
merged 7 commits into from Jun 29, 2017

Conversation

Projects
None yet
4 participants
@harshjain2
Member

harshjain2 commented Jun 24, 2017

Proposed changes that could improve perf.

@harshjain2 harshjain2 requested a review from codito Jun 24, 2017

@harshjain2 harshjain2 requested a review from smadala Jun 27, 2017

Show outdated Hide outdated src/Microsoft.TestPlatform.PlatformAbstractions/net46/Tracing/EqtTrace.cs
@@ -28,6 +28,11 @@ public partial class PlatformEqtTrace : IPlatformEqtTrace
/// <param name="childDomain">Child <c>AppDomain</c>.</param>
public void SetupRemoteEqtTraceListeners(AppDomain childDomain)
{
if(!isInitialized)

This comment has been minimized.

@smadala

smadala Jun 27, 2017

Member

As discussed with @AbhitejJohn , Verify that there is no side effects.

@smadala

smadala Jun 27, 2017

Member

As discussed with @AbhitejJohn , Verify that there is no side effects.

This comment has been minimized.

@harshjain2

harshjain2 Jun 27, 2017

Member

verified, no side effects :)

@harshjain2

harshjain2 Jun 27, 2017

Member

verified, no side effects :)

Show outdated Hide outdated src/Microsoft.TestPlatform.PlatformAbstractions/common/Tracing/EqtTrace.cs
case PlatformTraceLevel.Verbose:
return Source.Switch.ShouldTrace(TraceEventType.Verbose);
return isInitialized;

This comment has been minimized.

@smadala

smadala Jun 27, 2017

Member

Check isInitialized before calling ShouldTrace().

@smadala

smadala Jun 27, 2017

Member

Check isInitialized before calling ShouldTrace().

This comment has been minimized.

@harshjain2

harshjain2 Jun 27, 2017

Member

ShouldTrace is a public API. And I feel the right place is this method only.

@harshjain2

harshjain2 Jun 27, 2017

Member

ShouldTrace is a public API. And I feel the right place is this method only.

@harshjain2

This comment has been minimized.

Show comment
Hide comment
@harshjain2

harshjain2 Jun 27, 2017

Member

@dotnet-bot test Windows_NT / Release Build

Member

harshjain2 commented Jun 27, 2017

@dotnet-bot test Windows_NT / Release Build

Show outdated Hide outdated src/Microsoft.TestPlatform.PlatformAbstractions/common/Tracing/EqtTrace.cs
@@ -159,6 +159,11 @@ public bool InitializeVerboseTrace(string customLogFile)
/// <inheritdoc/>
public bool ShouldTrace(PlatformTraceLevel traceLevel)
{
if(!isInitialized)
@codito

codito approved these changes Jun 29, 2017

@harshjain2

This comment has been minimized.

Show comment
Hide comment
@harshjain2

harshjain2 Jun 29, 2017

Member

@dotnet-bot test Windows_NT / Debug Build

Member

harshjain2 commented Jun 29, 2017

@dotnet-bot test Windows_NT / Debug Build

@harshjain2 harshjain2 merged commit 730ec2e into Microsoft:master Jun 29, 2017

4 checks passed

Ubuntu16.04 / Debug Build Build finished.
Details
Ubuntu16.04 / Release Build Build finished.
Details
Windows_NT / Debug Build Build finished.
Details
Windows_NT / Release Build Build finished.
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment