Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.

Support large EventSource filter args #27500

Merged
merged 1 commit into from
Oct 29, 2019
Merged

Support large EventSource filter args #27500

merged 1 commit into from
Oct 29, 2019

Conversation

noahfalk
Copy link
Member

  1. Fix NullReferenceException. When the filter args exceeded the
    hard-coded size limit GetDataFromController would return data = null.
    The code previously asserted that data was not null and triggered NRE
    when it was. The fix correctly null checks the value instead of
    asserting and uses an empty args dictionary in this case, the same as if
    filter args had been empty to begin with.

  2. ETW has always limited filter args to 1024 bytes but EventPipe has no
    such restriction. When using DiagnosticSourceEventSource it can be
    useful to specify a larger filter arg blob. I can't do anything about
    ETW's restriction but there is no need for the runtime to force
    EventPipe to be equally limited. The larger size also reduces the chance
    that we need to hit the fallback path above causing filter args to be
    ignored.

@brianrob @sywhang @josalem - code review please

@noahfalk
Support large EventSource filter args
131412c 
1. Fix NullReferenceException. When the filter args exceeded the
hard-coded size limit GetDataFromController would return data = null.
The code previously asserted that data was not null and triggered NRE
when it was. The fix correctly null checks the value instead of
asserting and uses an empty args dictionary in this case, the same as if
filter args had been empty to begin with.

2. ETW has always limited filter args to 1024 bytes but EventPipe has no
such restriction. When using DiagnosticSourceEventSource it can be
useful to specify a larger filter arg blob. I can't do anything about
ETW's restriction but there is no need for the runtime to force
EventPipe to be equally limited. The larger size also reduces the chance
that we need to hit the fallback path above causing filter args to be
ignored.
josalem
josalem approved these changes Oct 28, 2019
View reviewed changes
Copy link

@josalem josalem left a comment

Choose a reason for hiding this comment

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

LGTM. To be clear, though, ETW still has the 1K limitation as part of it's internal mechanics despite the relaxed runtime check here?

@noahfalk
Copy link
Member Author

ETW still has the 1K limitation as part of it's internal mechanics despite the relaxed runtime check here?

Correct. We have no control over that part : )

@noahfalk noahfalk merged commit 857797d into dotnet:master Oct 29, 2019
@noahfalk noahfalk mentioned this pull request Oct 29, 2019
Merged
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corefx that referenced this pull request Oct 29, 2019
@noahfalk @dotnet-bot
Support large EventSource filter args (dotnet/coreclr#27500)
2c8ef5b 
1. Fix NullReferenceException. When the filter args exceeded the
hard-coded size limit GetDataFromController would return data = null.
The code previously asserted that data was not null and triggered NRE
when it was. The fix correctly null checks the value instead of
asserting and uses an empty args dictionary in this case, the same as if
filter args had been empty to begin with.

2. ETW has always limited filter args to 1024 bytes but EventPipe has no
such restriction. When using DiagnosticSourceEventSource it can be
useful to specify a larger filter arg blob. I can't do anything about
ETW's restriction but there is no need for the runtime to force
EventPipe to be equally limited. The larger size also reduces the chance
that we need to hit the fallback path above causing filter args to be
ignored.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/corert that referenced this pull request Oct 29, 2019
@noahfalk @dotnet-bot
Support large EventSource filter args (dotnet/coreclr#27500)
a0b29c2 
1. Fix NullReferenceException. When the filter args exceeded the
hard-coded size limit GetDataFromController would return data = null.
The code previously asserted that data was not null and triggered NRE
when it was. The fix correctly null checks the value instead of
asserting and uses an empty args dictionary in this case, the same as if
filter args had been empty to begin with.

2. ETW has always limited filter args to 1024 bytes but EventPipe has no
such restriction. When using DiagnosticSourceEventSource it can be
useful to specify a larger filter arg blob. I can't do anything about
ETW's restriction but there is no need for the runtime to force
EventPipe to be equally limited. The larger size also reduces the chance
that we need to hit the fallback path above causing filter args to be
ignored.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Dotnet-GitSync-Bot pushed a commit to Dotnet-GitSync-Bot/mono that referenced this pull request Oct 29, 2019
@noahfalk @dotnet-bot
Support large EventSource filter args (dotnet/coreclr#27500)
5fcde44 
1. Fix NullReferenceException. When the filter args exceeded the
hard-coded size limit GetDataFromController would return data = null.
The code previously asserted that data was not null and triggered NRE
when it was. The fix correctly null checks the value instead of
asserting and uses an empty args dictionary in this case, the same as if
filter args had been empty to begin with.

2. ETW has always limited filter args to 1024 bytes but EventPipe has no
such restriction. When using DiagnosticSourceEventSource it can be
useful to specify a larger filter arg blob. I can't do anything about
ETW's restriction but there is no need for the runtime to force
EventPipe to be equally limited. The larger size also reduces the chance
that we need to hit the fallback path above causing filter args to be
ignored.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
marek-safar pushed a commit to mono/mono that referenced this pull request Oct 29, 2019
@noahfalk @marek-safar
Support large EventSource filter args (dotnet/coreclr#27500)
67ff64a 
1. Fix NullReferenceException. When the filter args exceeded the
hard-coded size limit GetDataFromController would return data = null.
The code previously asserted that data was not null and triggered NRE
when it was. The fix correctly null checks the value instead of
asserting and uses an empty args dictionary in this case, the same as if
filter args had been empty to begin with.

2. ETW has always limited filter args to 1024 bytes but EventPipe has no
such restriction. When using DiagnosticSourceEventSource it can be
useful to specify a larger filter arg blob. I can't do anything about
ETW's restriction but there is no need for the runtime to force
EventPipe to be equally limited. The larger size also reduces the chance
that we need to hit the fallback path above causing filter args to be
ignored.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/corert that referenced this pull request Oct 30, 2019
@noahfalk @jkotas
Support large EventSource filter args (dotnet/coreclr#27500)
ee5d218 
1. Fix NullReferenceException. When the filter args exceeded the
hard-coded size limit GetDataFromController would return data = null.
The code previously asserted that data was not null and triggered NRE
when it was. The fix correctly null checks the value instead of
asserting and uses an empty args dictionary in this case, the same as if
filter args had been empty to begin with.

2. ETW has always limited filter args to 1024 bytes but EventPipe has no
such restriction. When using DiagnosticSourceEventSource it can be
useful to specify a larger filter arg blob. I can't do anything about
ETW's restriction but there is no need for the runtime to force
EventPipe to be equally limited. The larger size also reduces the chance
that we need to hit the fallback path above causing filter args to be
ignored.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
jkotas pushed a commit to dotnet/corefx that referenced this pull request Oct 30, 2019
@noahfalk @jkotas
Support large EventSource filter args (dotnet/coreclr#27500)
8f660fc 
1. Fix NullReferenceException. When the filter args exceeded the
hard-coded size limit GetDataFromController would return data = null.
The code previously asserted that data was not null and triggered NRE
when it was. The fix correctly null checks the value instead of
asserting and uses an empty args dictionary in this case, the same as if
filter args had been empty to begin with.

2. ETW has always limited filter args to 1024 bytes but EventPipe has no
such restriction. When using DiagnosticSourceEventSource it can be
useful to specify a larger filter arg blob. I can't do anything about
ETW's restriction but there is no need for the runtime to force
EventPipe to be equally limited. The larger size also reduces the chance
that we need to hit the fallback path above causing filter args to be
ignored.

Signed-off-by: dotnet-bot <dotnet-bot@microsoft.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@sywhang sywhang sywhang approved these changes

@josalem josalem josalem approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@noahfalk @josalem @sywhang