Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Expose HitCount and HitLineCount to CPUProfileNode #285

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mnutt
Copy link

@mnutt mnutt commented Feb 10, 2022

This exposes hitCount and hitLineCount from v8's cpuprofile node. It's useful in order to be able to see relative percentages of time spent in a v8 function and its descendants, although computing total time is left up to the presentation implementation.

Sample implementation:

func printTree(nest string, node *v8.CPUProfileNode) {
	file, name, line, col := node.GetScriptResourceName(), node.GetFunctionName(), node.GetLineNumber(), node.GetColumnNumber()
	selfHits, totalHits := node.GetHitCount(), totalHitCountForNode(node)

	fmt.Printf("[%d/%d hits\t]  %s%s() %s:%d:%d\n", selfHits, totalHits, nest, name, file, line, col)

	count := node.GetChildrenCount()
	if count == 0 {
		return
	}

	nest = fmt.Sprintf("%s  ", nest)

	for i := 0; i < count; i++ {
		printTree(nest, node.GetChild(i))
	}
}

func totalHitCountForNode(node *v8.CPUProfileNode) uint {
	totalHitCount := node.GetHitCount()

	count := node.GetChildrenCount()
	for i := 0; i < count; i++ {
		totalHitCount += totalHitCountForNode(node.GetChild(i))
	}

	return totalHitCount
}

@genevieve genevieve self-requested a review March 9, 2022 20:37
@@ -17,6 +17,12 @@ type CPUProfileNode struct {
// The number of the column where the function originates.
columnNumber int

// The number of samples recorded in the function
hitCount uint
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would you be open to using the comment from the v8 header file for these?

i.e.

// The count of samples where the function was currently executing.
hitCount uint
// The number of the function's source lines that collect the samples.
hitLineCount uint

// Returns total number of lines inside the function recording samples
func (c *CPUProfileNode) GetHitLineCount() uint {
return c.hitLineCount
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same for the getters ^

@genevieve
Copy link
Collaborator

The changes look good.

  1. Could you update the changelog to reflect these two new functions on nodes?
  2. Are there test assertions in the node suite that could cover these two new functions while also being reliable?

@codecov
Copy link

codecov bot commented Mar 9, 2022

Codecov Report

Merging #285 (99bdd18) into master (5e91d3d) will decrease coverage by 0.31%.
The diff coverage is 50.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #285      +/-   ##
==========================================
- Coverage   95.86%   95.54%   -0.32%     
==========================================
  Files          17       17              
  Lines         580      584       +4     
==========================================
+ Hits          556      558       +2     
- Misses         15       17       +2     
  Partials        9        9              
Impacted Files Coverage Δ
cpuprofilenode.go 77.77% <0.00%> (-22.23%) ⬇️
cpuprofiler.go 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e91d3d...99bdd18. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants