Skip to content

Commit

Permalink
Merge pull request #818 from vancem/FixComplexTraceLogging
Browse files Browse the repository at this point in the history 
Fix bug in a complex Self-describing ETW example
  • Loading branch information
@vancem
vancem authored Dec 13, 2018
2 parents 6caeac0 + ca8e9f4 commit b70bf91
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 14 deletions.
4 changes: 2 additions & 2 deletions src/Directory.Build.props
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@

<PropertyGroup>
<!-- These are the versions of the things we are CREATING in this repository -->
<PerfViewVersion>2.0.31</PerfViewVersion>
<TraceEventVersion>2.0.31</TraceEventVersion>
<PerfViewVersion>2.0.32</PerfViewVersion>
<TraceEventVersion>2.0.32</TraceEventVersion>
</PropertyGroup>

<!-- versions of dependencies that more than one project use -->
Expand Down
19 changes: 9 additions & 10 deletions src/TraceEvent/DynamicTraceEventParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -554,7 +554,7 @@ public override object PayloadValue(int index)
{
#if DEBUG
// Confirm that the serialization 'adds up'
var computedSize = SkipToField(payloadFetches, payloadFetches.Length, 0, EventDataLength);
var computedSize = SkipToField(payloadFetches, payloadFetches.Length, 0, EventDataLength, false);
Debug.Assert(computedSize <= this.EventDataLength);
if ((int)ID != 0xFFFE) // If it is not a manifest event
{
Expand All @@ -565,7 +565,7 @@ public override object PayloadValue(int index)
int offset = payloadFetches[index].Offset;
if (offset == ushort.MaxValue)
{
offset = SkipToField(payloadFetches, index, 0, EventDataLength);
offset = SkipToField(payloadFetches, index, 0, EventDataLength, true);
}

// Fields that are simply not present, (perfectly) we simply return null for.
Expand All @@ -574,7 +574,6 @@ public override object PayloadValue(int index)
return null;
}

// If we
return GetPayloadValueAt(ref payloadFetches[index], offset, EventDataLength);
}
catch (Exception e)
Expand Down Expand Up @@ -1073,21 +1072,21 @@ public override string GetFormattedMessage(IFormatProvider formatProvider)
#endregion

#region private
private int SkipToField(PayloadFetch[] payloadFetches, int targetFieldIdx, int startOffset, int payloadLength)
private int SkipToField(PayloadFetch[] payloadFetches, int targetFieldIdx, int startOffset, int payloadLength, bool useCache)
{
int fieldOffset;
int fieldIdx;

// First find a valid fieldIdx, fieldOffset pair
if (cachedEventId == EventIndex && cachedFieldIdx <= targetFieldIdx && startOffset == 0)
if (useCache && cachedEventId == EventIndex && cachedFieldIdx <= targetFieldIdx && startOffset == 0)
{
// We fetched a previous field, great, start from there.
fieldOffset = cachedFieldOffset;
fieldIdx = cachedFieldIdx;
}
else
{
// no cached value, search backwards for the the first field that has a fixed offset.
// no cached value, search backwards for the first field that has a fixed offset.
fieldOffset = 0;
fieldIdx = targetFieldIdx;
while (0 < fieldIdx)
Expand Down Expand Up @@ -1120,7 +1119,7 @@ private int SkipToField(PayloadFetch[] payloadFetches, int targetFieldIdx, int s
return payloadLength;
}

// however if we truely go past the end of the buffer, somethign went wrong and we wnat to signal that.
// however if we truly go past the end of the buffer, something went wrong and we want to signal that.
if (payloadLength < fieldOffset)
{
throw new ArgumentOutOfRangeException("Payload size exceeds buffer size.");
Expand All @@ -1130,14 +1129,14 @@ private int SkipToField(PayloadFetch[] payloadFetches, int targetFieldIdx, int s
}

// Remember our answer since can start there for the next field efficiently.
if (startOffset == 0)
if (useCache && startOffset == 0)
{
#if DEBUG
// If we computed the result using the cache, compute it again without the cache and we should get the same answer.
if (cachedEventId == this.EventIndex)
{
cachedEventId = EventIndex.Invalid;
Debug.Assert(fieldOffset == SkipToField(payloadFetches, targetFieldIdx, startOffset, payloadLength));
Debug.Assert(fieldOffset == SkipToField(payloadFetches, targetFieldIdx, startOffset, payloadLength, true));
}
#endif
cachedFieldOffset = fieldOffset;
Expand Down Expand Up @@ -1194,7 +1193,7 @@ internal int OffsetOfNextField(ref PayloadFetch payloadFetch, int offset, int pa
PayloadFetchClassInfo classInfo = payloadFetch.Class;
if (classInfo != null)
{
return SkipToField(classInfo.FieldFetches, classInfo.FieldFetches.Length, offset, payloadLength);
return SkipToField(classInfo.FieldFetches, classInfo.FieldFetches.Length, offset, payloadLength, false);
}

// TODO cache this when you parse the value so that you don't need to do it twice. Right now it is pretty inefficient.
Expand Down
4 changes: 2 additions & 2 deletions src/TraceEvent/RegisteredTraceEventParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1065,8 +1065,8 @@ private DynamicTraceEventData.PayloadFetchClassInfo ParseFields(int startField,
}

// is this dynamically sized with another field specifying the length?
// Is it an array?
if ((propertyInfo->Flags & (PROPERTY_FLAGS.ParamCount | PROPERTY_FLAGS.ParamLength)) != 0 || propertyInfo->InType == TdhInputType.Binary)
// Is it an array (binary and not a struct) (seems InType is not valid if property is a struct, so need to test for both.
if ((propertyInfo->Flags & (PROPERTY_FLAGS.ParamCount | PROPERTY_FLAGS.ParamLength)) != 0 || (propertyInfo->InType == TdhInputType.Binary && (propertyInfo->Flags & PROPERTY_FLAGS.Struct) == 0))
{
// silliness where if it is a byte[] they use Length otherwise they use count. Normalize it.
var countOrCountIndex = propertyInfo->CountOrCountIndex;
Expand Down

0 comments on commit b70bf91

Please sign in to comment.