Skip to content

Commit

Permalink
review suggestions implemented
Browse files Browse the repository at this point in the history
  • Loading branch information
@Maaarcocr
Maaarcocr committed Dec 18, 2017
1 parent 7f5dfe6 commit 45c060c
Show file tree
Hide file tree
Showing 4 changed files with 98 additions and 92 deletions.
21 changes: 6 additions & 15 deletions internal/driver/testdata/pprof.cpu.flat.addresses.weblist
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,20 +38,11 @@ background-color: #eeeeee;
color: #008800;
display: none;
}
.percentile_80 {
.ptile_95 {
background-color: #fe5252
}
.percentile_60 {
background-color: #e09764
}
.percentile_40 {
background-color: #d1b96d
}
.percentile_20 {
background-color: #c3dc76
}
.percentile_10 {
background-color: #b3ff80
.ptile_80 {
background-color: #ffb6c1
}
</style>
<script type="text/javascript">
Expand Down Expand Up @@ -79,7 +70,7 @@ Type: cpu<br>
Duration: 10s, Total samples = 1.12s (11.20%)<br>Total: 1.12s</div><h2>line1000</h2><p class="filename">testdata/file1000.src</p>
<pre onClick="pprof_toggle_asm(event)">
Total: 1.10s 1.10s (flat, cum) 98.21%
<span class=line> 1</span> <span class="deadsrc percentile_80"> 1.10s 1.10s line1 </span><span class=asm> 1.10s 1.10s 1000: instruction one <span class=unimportant>file1000.src:1</span>
<span class=line> 1</span> <span class="deadsrc ptile_80"> 1.10s 1.10s line1 </span><span class=asm> 1.10s 1.10s 1000: instruction one <span class=unimportant>file1000.src:1</span>
. . 1001: instruction two <span class=unimportant>file1000.src:1</span>
⋮
. . 1003: instruction four <span class=unimportant>file1000.src:1</span>
Expand All @@ -100,11 +91,11 @@ Duration: 10s, Total samples = 1.12s (11.20%)<br>Total: 1.12s</div><h2>line1000<
<span class=line> 3</span> <span class="nop"> . . line3 </span>
<span class=line> 4</span> <span class="nop"> . . line4 </span>
<span class=line> 5</span> <span class="nop"> . . line5 </span>
<span class=line> 6</span> <span class="deadsrc percentile_80"> 10ms 1.01s line6 </span><span class=asm> 10ms 1.01s 3000: instruction one <span class=unimportant>file3000.src:6</span>
<span class=line> 6</span> <span class="deadsrc ptile_80"> 10ms 1.01s line6 </span><span class=asm> 10ms 1.01s 3000: instruction one <span class=unimportant>file3000.src:6</span>
</span>
<span class=line> 7</span> <span class="nop"> . . line7 </span>
<span class=line> 8</span> <span class="nop"> . . line8 </span>
<span class=line> 9</span> <span class="deadsrc percentile_60"> . 110ms line9 </span><span class=asm> . 100ms 3001: instruction two <span class=unimportant>file3000.src:9</span>
<span class=line> 9</span> <span class="deadsrc ptile_80"> . 110ms line9 </span><span class=asm> . 100ms 3001: instruction two <span class=unimportant>file3000.src:9</span>
. 10ms 3002: instruction three <span class=unimportant>file3000.src:9</span>
. . 3003: instruction four <span class=unimportant></span>
. . 3004: instruction five <span class=unimportant></span>
Expand Down
71 changes: 32 additions & 39 deletions internal/report/source.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -215,71 +215,64 @@ func PrintWebList(w io.Writer, rpt *Report, obj plugin.ObjTool, maxFiles int) er
}

printFunctionHeader(w, ff.functionName, path, n.Flat, n.Cum, rpt)
percentiles := calculatePercentiles(fnodes)
ptiles := calculatePtiles(fnodes)
for _, fn := range fnodes {
printFunctionSourceLine(w, fn, asm[fn.Info.Lineno], reader, getPercentileString(float64(fn.Cum), percentiles), rpt)
printFunctionSourceLine(w, fn, asm[fn.Info.Lineno], reader, getPtileCSSClassName(fn.Cum, ptiles), rpt)
}
printFunctionClosing(w)
}
return nil
}

func getPercentileString(cumSum float64, percentiles map[float64]float64) string {
func getPtileCSSClassName(cumSum int64, ptiles map[int64]int64) string {
if cumSum == 0 {
return ""
}
switch {
case cumSum >= percentiles[80]:
return " percentile_80"
case cumSum >= percentiles[60]:
return " percentile_60"
case cumSum >= percentiles[40]:
return " percentile_40"
case cumSum >= percentiles[20]:
return " percentile_20"
case cumSum >= percentiles[10]:
return " percentile_10"
for key, value := range ptiles {
if cumSum > value {
return " ptile_" + strconv.FormatInt(key, 10)
}
}
return ""
}

// calculate percentile expects cumSums to be sorted and
// to contain unique elements. It also expects the percentile to be between 0 and 99.
func calculatePercentile(percentile float64, cumSums []float64) float64 {
rank := percentile / 100 * float64(len(cumSums))
// calculatePtile expects cumSums to be sorted and
// to contain unique elements. It also expects the ptile to be between 0 and 99.
func calculatePtile(ptile int64, cumSums []int64) int64 {
rank := float64(ptile) / 100 * float64(len(cumSums))
return cumSums[int64(rank)]
}

func getSetOfCumValues(fnodes graph.Nodes) []float64 {
mapOfCumValues := make(map[int64]bool)
func getArrayOfCumValues(fnodes graph.Nodes) []int64 {
arrayOfCumValues := make([]int64, 0, len(fnodes))

for _, fn := range fnodes {
if _, ok := mapOfCumValues[fn.Cum]; !ok {
mapOfCumValues[fn.Cum] = true
}
arrayOfCumValues = append(arrayOfCumValues, fn.Cum)
}

setOfCumValues := make([]float64, 0, len(mapOfCumValues))
for key := range mapOfCumValues {
setOfCumValues = append(setOfCumValues, float64(key))
}
return setOfCumValues
return arrayOfCumValues
}

// calculatePercentiles returns nil when the fnodes is 0
type int64Slice []int64

func (a int64Slice) Len() int { return len(a) }
func (a int64Slice) Swap(i, j int) { a[i], a[j] = a[j], a[i] }
func (a int64Slice) Less(i, j int) bool { return a[i] < a[j] }

// calculatePtiles returns nil when the fnodes is 0
// because its result will never be used in such a case.
func calculatePercentiles(fnodes graph.Nodes) map[float64]float64 {
func calculatePtiles(fnodes graph.Nodes) map[int64]int64 {
if len(fnodes) == 0 {
return nil
}
setOfCumValues := getSetOfCumValues(fnodes)
percentiles := map[float64]float64{80: 0, 60: 0, 40: 0, 20: 0, 10: 0}
sort.Float64s(setOfCumValues)
for key := range percentiles {
percentiles[key] = calculatePercentile(key, setOfCumValues)
arrayOfCumValues := getArrayOfCumValues(fnodes)
ptiles := map[int64]int64{95: 0, 80: 0}
sort.Sort(int64Slice(arrayOfCumValues))
for key := range ptiles {
ptiles[key] = calculatePtile(key, arrayOfCumValues)
}

return percentiles
return ptiles
}

// sourceCoordinates returns the lowest and highest line numbers from
Expand Down Expand Up @@ -408,12 +401,12 @@ func printFunctionHeader(w io.Writer, name, path string, flatSum, cumSum int64,
}

// printFunctionSourceLine prints a source line and the corresponding assembly.
func printFunctionSourceLine(w io.Writer, fn *graph.Node, assembly []assemblyInstruction, reader *sourceReader, percentileString string, rpt *Report) {
func printFunctionSourceLine(w io.Writer, fn *graph.Node, assembly []assemblyInstruction, reader *sourceReader, ptileCSSClassName string, rpt *Report) {
if len(assembly) == 0 {
fmt.Fprintf(w,
"<span class=line> %6d</span> <span class=\"nop%s\"> %10s %10s %8s %s </span>\n",
fn.Info.Lineno,
percentileString,
ptileCSSClassName,
valueOrDot(fn.Flat, rpt), valueOrDot(fn.Cum, rpt),
"", template.HTMLEscapeString(fn.Info.Name))
return
Expand All @@ -422,7 +415,7 @@ func printFunctionSourceLine(w io.Writer, fn *graph.Node, assembly []assemblyIns
fmt.Fprintf(w,
"<span class=line> %6d</span> <span class=\"deadsrc%s\"> %10s %10s %8s %s </span>",
fn.Info.Lineno,
percentileString,
ptileCSSClassName,
valueOrDot(fn.Flat, rpt), valueOrDot(fn.Cum, rpt),
"", template.HTMLEscapeString(fn.Info.Name))
srcIndent := indentation(fn.Info.Name)
Expand Down
15 changes: 3 additions & 12 deletions internal/report/source_html.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -58,20 +58,11 @@ background-color: #eeeeee;
color: #008800;
display: none;
}
.percentile_80 {
.ptile_95 {
background-color: #fe5252
}
.percentile_60 {
background-color: #e09764
}
.percentile_40 {
background-color: #d1b96d
}
.percentile_20 {
background-color: #c3dc76
}
.percentile_10 {
background-color: #b3ff80
.ptile_80 {
background-color: #ffb6c1
}
</style>`

Expand Down
83 changes: 57 additions & 26 deletions internal/report/source_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import (
"reflect"
"regexp"
"runtime"
"strconv"
"strings"
"testing"

Expand All @@ -15,43 +16,73 @@ import (
"github.com/google/pprof/profile"
)

func TestCalculatePercentiles(t *testing.T) {
type prettyNodes graph.Nodes

func (this prettyNodes) String() string {
strs := make([]string, 0, len(this)+2)
strs = append(strs, "[")
for _, node := range this {
strs = append(strs, "Cum: "+strconv.FormatInt(node.Cum, 10))
}
strs = append(strs, "]")
return strings.Join(strs, " ")
}

func TestCalculatePtiles(t *testing.T) {
for _, testCase := range []struct {
fnodes graph.Nodes
output map[float64]float64
nodes graph.Nodes
want map[int64]int64
}{
{
fnodes: []*graph.Node{},
output: nil,
nodes: nil,
want: nil,
},
{
nodes: []*graph.Node{
{Cum: 0},
{Cum: 8},
},
want: map[int64]int64{80: 8, 95: 8},
},
{
nodes: []*graph.Node{
{Cum: 10},
{Cum: 5},
{Cum: 8},
},
want: map[int64]int64{80: 10, 95: 10},
},
{
fnodes: []*graph.Node{
{
Cum: 0,
},
{
Cum: 8,
},
nodes: []*graph.Node{
{Cum: 0},
{Cum: 0},
{Cum: 0},
{Cum: 0},
{Cum: 11},
{Cum: 2},
{Cum: 9},
{Cum: 4},
{Cum: 8},
{Cum: 2},
{Cum: 16},
{Cum: 22},
{Cum: 13},
},
output: map[float64]float64{10: 0, 20: 0, 40: 0, 60: 8, 80: 8},
want: map[int64]int64{80: 13, 95: 22},
},
{
fnodes: []*graph.Node{
{
Cum: 10,
},
{
Cum: 5,
},
{
Cum: 8,
},
nodes: []*graph.Node{
{Cum: 10},
{Cum: 10},
{Cum: 10},
{Cum: 10},
{Cum: 10},
},
output: map[float64]float64{10: 5, 20: 5, 40: 8, 60: 8, 80: 10},
want: map[int64]int64{80: 10, 95: 10},
},
} {
if percentiles := calculatePercentiles(testCase.fnodes); !reflect.DeepEqual(percentiles, testCase.output) {
t.Errorf("%v is not equal to %v", percentiles, testCase.output)
if ptiles := calculatePtiles(testCase.nodes); !reflect.DeepEqual(ptiles, testCase.want) {
t.Errorf("calculatePtiles(%v) = %v; want %v", prettyNodes(testCase.nodes), ptiles, testCase.want)
}
}
}
Expand Down

0 comments on commit 45c060c

Please sign in to comment.