Enable heap prof of stmgr in heron-shell. #2005
Conversation
|
What are the performance implications of doing this? Do we want to do this only when DEBUG is set? |
|
Before we merge this pr into master, let's test with some user topologies to see if there's any performance degradation. For debugging the issue, we can make a customized release based on the branch. |
|
Can we come up with a plan to carefully evaluate the performance implications of this? Stmgr is very performance-critical. |
| @@ -55,6 +56,8 @@ int main(int argc, char* argv[]) { | |||
| sp_int32 shell_port = atoi(argv[11]); | |||
| sp_string heron_internals_config_filename = argv[12]; | |||
|
|
|||
| HeapProfilerStart("stmgr"); | |||
There was a problem hiding this comment.
The profiling is enabled during the whole stmgr life time.
What about adding two handlers to the stmgr server to control the profiling start & end:
- HandleStartHeapProfilingMessage: HeapProfilerStart()
- HandleStopHeapProfilingMessage: HeapProfilerStop()
In this way, the profiling is not enabled by default and there is no impact on performance.
|
I searched around the web but found no official comment on overhead. Several blog posts say enabling heap profiling introduces little overhead. I have also tested on @ttim's large devel topology and I see no performance degradation. This PR simply adds shell support. For tracker and UI support, we can do in another patch. |
|
Any comment on this? |
|
@objmagic One easy check that you can do is run the exclamation topology both with and without this change and see what kind of impact you see. |
|
@srkukarni Agree. I am on PTO and will find time to do so. Also, exclamation topology is throttled now so I will try word count one instead. |
|
Any updates on this PR ? We saw stmgr out of memory on 0.15.1 version again. If the stmgr memory profiling is enabled, it should be easier to debug stmgr out of memory issue. What about adding two handlers to the stmgr server to control the profiling start & end: Besides, this profiling tool cannot tell which stmgr part code calls new_string(), new_string() occupied more than 60% of total stmgr memory if I remember correctly. @objmagic @srkukarni thoughts? |
Tested locally by
curlheron-shell.Normally stmgr uses <500MB, but in one of our prod topologies, some stmgrs are using >1.5GB memory which causes container gets killed. We suspect mempool is not capped but we'd like to do some heap profiling first.