Skip to content

Commit 234e004

Browse files
nolanmar511nolanmar511
committed
address comments
1 parent bbec955 commit 234e004

File tree

4 files changed

+74
-87
lines changed

4 files changed

+74
-87
lines changed

‎internal/report/report.go

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ type Options struct {
8383

8484
// Generate generates a report as directed by the Report.
8585
func Generate(w io.Writer, rpt *Report, obj plugin.ObjTool, ui plugin.UI) error {
86-
if rpt.unexpNegSamples {
86+
if rpt.hasNegativeSamples {
8787
ui.PrintErr("Negative sample values appeared in the profile.\nIf using the -base flag to compare profiles, consider using the -diff_base flag instead.")
8888
}
8989

@@ -1206,8 +1206,8 @@ func New(prof *profile.Profile, o *Options) *Report {
12061206
}
12071207
return measurement.ScaledLabel(v, o.SampleUnit, o.OutputUnit)
12081208
}
1209-
total, unexpNegSamples := computeTotal(prof, o.SampleValue, o.SampleMeanDivisor)
1210-
return &Report{prof, total, o, format, unexpNegSamples}
1209+
total, hasNegativeSamples := computeTotal(prof, o.SampleValue, o.SampleMeanDivisor)
1210+
return &Report{prof, total, hasNegativeSamples, o, format}
12111211
}
12121212

12131213
// NewDefault builds a new report indexing the last sample value
@@ -1266,11 +1266,11 @@ func computeTotal(prof *profile.Profile, value, meanDiv func(v []int64) int64) (
12661266
// Report contains the data and associated routines to extract a
12671267
// report from a profile.
12681268
type Report struct {
1269-
prof *profile.Profile
1270-
total int64
1271-
options *Options
1272-
formatValue func(int64) string
1273-
unexpNegSamples bool
1269+
prof *profile.Profile
1270+
total int64
1271+
hasNegativeSamples bool
1272+
options *Options
1273+
formatValue func(int64) string
12741274
}
12751275

12761276
// Total returns the total number of samples in a report.

‎internal/report/report_test.go

Lines changed: 29 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -39,52 +39,54 @@ func TestSource(t *testing.T) {
3939
sampleValue1 := func(v []int64) int64 {
4040
return v[1]
4141
}
42-
42+
negProfile := testProfile.Copy()
43+
for _, sample := range negProfile.Sample {
44+
for i, v := range sample.Value {
45+
sample.Value[i] = -v
46+
}
47+
}
4348
for _, tc := range []testcase{
4449
{
4550
rpt: New(
46-
testProfile1.Copy(),
51+
testProfile.Copy(),
4752
&Options{
4853
OutputFormat: List,
4954
Symbol: regexp.MustCompile(`.`),
5055
TrimPath: "/some/path",
5156

5257
SampleValue: sampleValue1,
53-
SampleUnit: testProfile1.SampleType[1].Unit,
58+
SampleUnit: testProfile.SampleType[1].Unit,
5459
},
5560
),
5661
want: path + "source.rpt",
5762
},
5863
{
5964
rpt: New(
60-
testProfile1.Copy(),
65+
testProfile.Copy(),
6166
&Options{
6267
OutputFormat: Dot,
6368
CallTree: true,
6469
Symbol: regexp.MustCompile(`.`),
6570
TrimPath: "/some/path",
6671

6772
SampleValue: sampleValue1,
68-
SampleUnit: testProfile1.SampleType[1].Unit,
73+
SampleUnit: testProfile.SampleType[1].Unit,
6974
},
7075
),
7176
want: path + "source.dot",
7277
},
7378
{
7479
rpt: New(
75-
testProfile2.Copy(),
80+
negProfile.Copy(),
7681
&Options{
77-
OutputFormat: Dot,
78-
CallTree: true,
82+
OutputFormat: List,
7983
Symbol: regexp.MustCompile(`.`),
80-
TrimPath: "/some/path",
81-
82-
SampleValue: sampleValue1,
83-
SampleUnit: testProfile2.SampleType[1].Unit,
84+
SampleValue: sampleValue1,
85+
SampleUnit: negProfile.SampleType[1].Unit,
8486
},
8587
),
8688
wantNegSampleErr: true,
87-
want: path + "source_neg_samples.dot",
89+
want: path + "source_neg_samples.rpt",
8890
},
8991
} {
9092
var b bytes.Buffer
@@ -203,7 +205,7 @@ var testL = []*profile.Location{
203205
},
204206
}
205207

206-
var testProfile1 = &profile.Profile{
208+
var testProfile = &profile.Profile{
207209
PeriodType: &profile.ValueType{Type: "cpu", Unit: "millisecond"},
208210
Period: 10,
209211
DurationNanos: 10e9,
@@ -238,41 +240,6 @@ var testProfile1 = &profile.Profile{
238240
Mapping: testM,
239241
}
240242

241-
var testProfile2 = &profile.Profile{
242-
PeriodType: &profile.ValueType{Type: "cpu", Unit: "millisecond"},
243-
Period: 10,
244-
DurationNanos: 10e9,
245-
SampleType: []*profile.ValueType{
246-
{Type: "samples", Unit: "count"},
247-
{Type: "cpu", Unit: "cycles"},
248-
},
249-
Sample: []*profile.Sample{
250-
{
251-
Location: []*profile.Location{testL[0]},
252-
Value: []int64{1, 1},
253-
},
254-
{
255-
Location: []*profile.Location{testL[2], testL[1], testL[0]},
256-
Value: []int64{1, 10},
257-
},
258-
{
259-
Location: []*profile.Location{testL[4], testL[2], testL[0]},
260-
Value: []int64{1, -100},
261-
},
262-
{
263-
Location: []*profile.Location{testL[3], testL[0]},
264-
Value: []int64{1, 1000},
265-
},
266-
{
267-
Location: []*profile.Location{testL[4], testL[3], testL[0]},
268-
Value: []int64{1, 10000},
269-
},
270-
},
271-
Location: testL,
272-
Function: testF,
273-
Mapping: testM,
274-
}
275-
276243
func TestDisambiguation(t *testing.T) {
277244
parent1 := &graph.Node{Info: graph.NodeInfo{Name: "parent1"}}
278245
parent2 := &graph.Node{Info: graph.NodeInfo{Name: "parent2"}}
@@ -356,7 +323,7 @@ func TestLegendActiveFilters(t *testing.T) {
356323
}
357324

358325
func TestComputeTotal(t *testing.T) {
359-
p1 := testProfile1.Copy()
326+
p1 := testProfile.Copy()
360327
p1.Sample = []*profile.Sample{
361328
{
362329
Location: []*profile.Location{testL[0]},
@@ -372,7 +339,7 @@ func TestComputeTotal(t *testing.T) {
372339
},
373340
}
374341

375-
p2 := testProfile1.Copy()
342+
p2 := testProfile.Copy()
376343
p2.Sample = []*profile.Sample{
377344
{
378345
Location: []*profile.Location{testL[0]},
@@ -388,7 +355,7 @@ func TestComputeTotal(t *testing.T) {
388355
},
389356
}
390357

391-
p3 := testProfile1.Copy()
358+
p3 := testProfile.Copy()
392359
p3.Sample = []*profile.Sample{
393360
{
394361
Location: []*profile.Location{testL[0]},
@@ -425,11 +392,11 @@ func TestComputeTotal(t *testing.T) {
425392
}
426393

427394
testcases := []struct {
428-
desc string
429-
prof *profile.Profile
430-
value, meanDiv func(v []int64) int64
431-
wantTotal int64
432-
wantUnexpNegSamples bool
395+
desc string
396+
prof *profile.Profile
397+
value, meanDiv func(v []int64) int64
398+
wantTotal int64
399+
wantHasNegativeSamples bool
433400
}{
434401
{
435402
desc: "no diff base, all positive values, index 1",
@@ -453,8 +420,8 @@ func TestComputeTotal(t *testing.T) {
453420
value: func(v []int64) int64 {
454421
return v[1]
455422
},
456-
wantTotal: 111,
457-
wantUnexpNegSamples: true,
423+
wantTotal: 111,
424+
wantHasNegativeSamples: true,
458425
},
459426
{
460427
desc: "diff base, some negative values",
@@ -468,12 +435,12 @@ func TestComputeTotal(t *testing.T) {
468435

469436
for _, tc := range testcases {
470437
t.Run(tc.desc, func(t *testing.T) {
471-
gotTotal, gotUnexpNegSamples := computeTotal(tc.prof, tc.value, tc.meanDiv)
438+
gotTotal, gotHasNegativeSamples := computeTotal(tc.prof, tc.value, tc.meanDiv)
472439
if gotTotal != tc.wantTotal {
473440
t.Errorf("got total %d, want %v", gotTotal, tc.wantTotal)
474441
}
475-
if gotUnexpNegSamples != tc.wantUnexpNegSamples {
476-
t.Errorf("got unexpected negative samples %v, want %v", gotUnexpNegSamples, tc.wantUnexpNegSamples)
442+
if gotHasNegativeSamples != tc.wantHasNegativeSamples {
443+
t.Errorf("got unexpected negative samples %v, want %v", gotHasNegativeSamples, tc.wantHasNegativeSamples)
477444
}
478445
})
479446
}

‎internal/report/testdata/source_neg_samples.dot

Lines changed: 0 additions & 17 deletions
This file was deleted.

‎internal/report/testdata/source_neg_samples.rpt

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
Total: 11111
2+
ROUTINE ======================== bar in testdata/source1
3+
-10 -110 (flat, cum) 0.99% of Total
4+
. . 5:source1 line 5;
5+
. . 6:source1 line 6;
6+
. . 7:source1 line 7;
7+
. . 8:source1 line 8;
8+
. . 9:source1 line 9;
9+
-10 -110 10:source1 line 10;
10+
. . 11:source1 line 11;
11+
. . 12:source1 line 12;
12+
. . 13:source1 line 13;
13+
. . 14:source1 line 14;
14+
. . 15:source1 line 15;
15+
ROUTINE ======================== foo in testdata/source1
16+
0 -10 (flat, cum) 0.09% of Total
17+
. . 1:source1 line 1;
18+
. . 2:source1 line 2;
19+
. . 3:source1 line 3;
20+
. -10 4:source1 line 4;
21+
. . 5:source1 line 5;
22+
. . 6:source1 line 6;
23+
. . 7:source1 line 7;
24+
. . 8:source1 line 8;
25+
. . 9:source1 line 9;
26+
ROUTINE ======================== main in testdata/source1
27+
-1 -11111 (flat, cum) 100% of Total
28+
. . 1:source1 line 1;
29+
-1 -11111 2:source1 line 2;
30+
. . 3:source1 line 3;
31+
. . 4:source1 line 4;
32+
. . 5:source1 line 5;
33+
. . 6:source1 line 6;
34+
. . 7:source1 line 7;
35+
ROUTINE ======================== tee in /some/path/testdata/source2
36+
-11100 -21100 (flat, cum) 189.90% of Total
37+
Error: open /some/path/testdata/source2: no such file or directory

0 commit comments

Comments
 (0)