Skip to content

Commit

Permalink
fix unit test release handling
Browse files Browse the repository at this point in the history
  • Loading branch information
thorfour committed Feb 22, 2024
1 parent c448598 commit 20517a8
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 14 deletions.
8 changes: 4 additions & 4 deletions pkg/parcacol/arrow.go
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ func (c *ArrowToProfileConverter) Convert(
return profile.OldProfile{}, ErrMissingColumn{Column: "locations", Columns: len(indices)}
}
locations := ar.Column(indices[0]).(*array.List)
locationOffsets := locations.Offsets()
locationOffsets := locations.Offsets()[locations.Offset() : locations.Offset()+1+locations.Len()] // Adjust offsets by the data offset. This happens if this list is a slice of a larger list.
location := locations.ListValues().(*array.Struct)
address := location.Field(0).(*array.Uint64)
mappingStart := location.Field(1).(*array.Uint64)
Expand All @@ -82,7 +82,7 @@ func (c *ArrowToProfileConverter) Convert(
mappingBuildID := location.Field(5).(*array.Dictionary)
mappingBuildIDDict := mappingBuildID.Dictionary().(*array.Binary)
lines := location.Field(6).(*array.List)
lineOffsets := lines.Offsets()
lineOffsets := lines.Offsets()[lines.Offset() : lines.Offset()+1+lines.Len()] // Adjust offsets by the data offset. This happens if this list is a slice of a larger list.
line := lines.ListValues().(*array.Struct)
lineNumber := line.Field(0).(*array.Int64)
lineFunctionName := line.Field(1).(*array.Dictionary)
Expand Down Expand Up @@ -125,8 +125,8 @@ func (c *ArrowToProfileConverter) Convert(
}
}

lOffsetStart := locationOffsets[i+locations.Offset()]
lOffsetEnd := locationOffsets[i+1+locations.Offset()]
lOffsetStart := locationOffsets[i]
lOffsetEnd := locationOffsets[i+1]
stacktrace := make([]*profile.Location, 0, lOffsetEnd-lOffsetStart)
for j := int(lOffsetStart); j < int(lOffsetEnd); j++ {
llOffsetStart := lineOffsets[j]
Expand Down
11 changes: 6 additions & 5 deletions pkg/query/columnquery.go
Original file line number Diff line number Diff line change
Expand Up @@ -275,11 +275,6 @@ func (q *ColumnQueryAPI) Query(ctx context.Context, req *pb.QueryRequest) (*pb.Q
if err != nil {
return nil, fmt.Errorf("filtering profile: %w", err)
}
defer func() {
for _, r := range p.Samples {
r.Release()
}
}()

return q.renderReport(
ctx,
Expand All @@ -305,6 +300,12 @@ func FilterProfileData(
_, span := tracer.Start(ctx, "filterByFunction")
defer span.End()

defer func() {
for _, r := range records {
r.Release()
}
}()

// We want to filter by function name case-insensitive, so we need to lowercase the query.
// We lower case the query here, so we don't have to do it for every sample.
filterQueryBytes := []byte(strings.ToLower(filterQuery))
Expand Down
25 changes: 20 additions & 5 deletions pkg/query/columnquery_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1473,7 +1473,6 @@ func TestFilterData(t *testing.T) {
w.Diff.Append(0)

originalRecord := w.RecordBuilder.NewRecord()
defer originalRecord.Release()
recs, _, err := FilterProfileData(
context.Background(),
noop.NewTracerProvider().Tracer(""),
Expand All @@ -1485,6 +1484,11 @@ func TestFilterData(t *testing.T) {
},
)
require.NoError(t, err)
defer func() {
for _, r := range recs {
r.Release()
}
}()
r := profile.NewRecordReader(recs[0])
valid := 0
for i := 0; i < r.Location.Len(); i++ {
Expand Down Expand Up @@ -1552,7 +1556,6 @@ func TestFilterDataWithPath(t *testing.T) {
w.Diff.Append(0)

originalRecord := w.RecordBuilder.NewRecord()
defer originalRecord.Release()
recs, _, err := FilterProfileData(
context.Background(),
noop.NewTracerProvider().Tracer(""),
Expand All @@ -1562,6 +1565,11 @@ func TestFilterDataWithPath(t *testing.T) {
nil,
)
require.NoError(t, err)
defer func() {
for _, r := range recs {
r.Release()
}
}()
r := profile.NewRecordReader(recs[0])
valid := 0
for i := 0; i < r.Location.Len(); i++ {
Expand Down Expand Up @@ -1629,7 +1637,6 @@ func TestFilterDataInterpretedOnly(t *testing.T) {
w.Diff.Append(0)

originalRecord := w.RecordBuilder.NewRecord()
defer originalRecord.Release()
recs, _, err := FilterProfileData(
context.Background(),
noop.NewTracerProvider().Tracer(""),
Expand All @@ -1641,6 +1648,11 @@ func TestFilterDataInterpretedOnly(t *testing.T) {
},
)
require.NoError(t, err)
defer func() {
for _, r := range recs {
r.Release()
}
}()
r := profile.NewRecordReader(recs[0])
valid := 0
for i := 0; i < r.Location.Len(); i++ {
Expand Down Expand Up @@ -1710,9 +1722,9 @@ func BenchmarkFilterData(t *testing.B) {

originalRecord := w.RecordBuilder.NewRecord()
defer originalRecord.Release()

for i := 0; i < t.N; i++ {
_, _, err := FilterProfileData(
originalRecord.Retain() // retain each time since FilterProfileData will release it
recs, _, err := FilterProfileData(
context.Background(),
noop.NewTracerProvider().Tracer(""),
mem,
Expand All @@ -1723,5 +1735,8 @@ func BenchmarkFilterData(t *testing.B) {
},
)
require.NoError(t, err)
for _, r := range recs {
r.Release()
}
}
}

0 comments on commit 20517a8

Please sign in to comment.