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

Remove timeout when finishing traces #4

Conversation

juanperi
Copy link
Contributor

@juanperi juanperi commented Oct 27, 2021

Hey, while giving eflambe a try, i hit the timeouts mentioned in #3
As it was mentioned there, I just added infinity to the genserver calls when finishing the traces, and then the flame graphs were generated properly

Closes #3

@Stratus3D
Copy link
Owner

Thanks for the PR @juanperi! This is will get around the issue, but I'd rather not have to set the timeout to infinity. I'd rather update the code so less process has to be done in the finalize callback. Instead of doing all the work in the finalize callback I'd rather have the formatter do as much work as possible in handle_trace_event that is invoked for each event and incrementally build the formatted trace output. This will take some refactoring.

For now though this solution is a sufficient workaround. I am going to keep #3 open however.

@Stratus3D Stratus3D merged commit dd5bd54 into Stratus3D:master Oct 27, 2021
@juanperi
Copy link
Contributor Author

hey, thank you!
i have a question though, if you do more work in every trace, as opposed to doing it in the finalize, won't it interfere more with the system where the trace is running while the trace is running?
I understand it is a symplistic view where we trace one thing at a time, but though to ask anyways

@juanperi juanperi deleted the fix/remove-timeout-when-finishing-the-traces branch October 27, 2021 19:28
@Stratus3D
Copy link
Owner

@juanperi that's a really good point. Since eflambe is running on the same node and server, anything running in eflambe could have an affect on the code being profiled. There isn't really a way around that with a single node. We could support running traces from a remote node to reduce this, but that currently isn't supported. With the way the code works right now, eflambe holds all those events in memory until formatting and writing them to disk at the end. So the current approach is pretty bad when it comes to memory usage, as usage increases infinitely until the trace is finished. Whereas writing incrementally would make memory usage more constant throughout the profiling. It's a tradeoff of memory vs CPU. Which is more important depends on the situation, and both will have an affect on performance.

@juanperi
Copy link
Contributor Author

@Stratus3D all very good points.
For the simplistic use case I tried this, I would say saving CPU is better than saving memory. Because I was running it in a single node in local, to get sequential information of a single request.

I'll follow the other issue, and see what direction you want to take it.

Thank you!!

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.

Fix timeout issue when formatting large traces
2 participants