-
Notifications
You must be signed in to change notification settings - Fork 1
ddprof Cgo test case #3
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
base: main
Are you sure you want to change the base?
Conversation
| { | ||
| "regular_expression": ".*runtime\\.asmcgocall\\.abi0;cAllocateMemory", | ||
| "percent": 20, | ||
| "error_margin": 40, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't really care about the numbers (I can't predict the mmap strategy from Go), I just want them to be there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you call this out more clearly in the README
Add a test case mixing Go and C allocations
Add documentation to comment on test purpose
3c19c2e to
df91785
Compare
| { | ||
| "regular_expression": ".*runtime\\.asmcgocall\\.abi0;cAllocateMemory", | ||
| "percent": 20, | ||
| "error_margin": 40, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you call this out more clearly in the README
| && rm -rf /var/lib/apt/lists/* | ||
|
|
||
| # Install native profiling | ||
| ARG CACHE_DATE=2023-03-01_09:58:27 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Noob question: What does this date signify? Does it mean we're using a version from March?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a trick to force the second part of the docker to re-fetch new versions of the profiler.
| for (int i = 0; i < size_alloc; i++) { | ||
| sum += data[i]; | ||
| } | ||
| printf("%d\n", sum); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
⛏️ indentation
| func burnCPU() { | ||
| // Simulate CPU-intensive work | ||
| for i := 0; i < 1000000000; i++ { | ||
| _ = i * i |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How do we guarantee this won't be optimized?
|
|
||
| func allocateMemory() { | ||
| // Allocate some memory in Go | ||
| data := make([]int, 100) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a reason we're doing 10K in C, and 100 in Go? For future, might be interesting to make this configurable to see how we do at profiling as the relative ratios change
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really, only that I was sure not to catch these for now, so I did not care about having smaller allocations. The native profiler can not instrument the Go allocator (only the mmaps).
We can unify if you feel it is more logical.
Description
Add a test case mixing Go and C allocations