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

Introduce memory dump support #5

Open
rburchell opened this issue Mar 31, 2017 · 3 comments
Open

Introduce memory dump support #5

rburchell opened this issue Mar 31, 2017 · 3 comments

Comments

@rburchell
Copy link
Member

There's some ifdef'd out code in traced that dumps memory info into the JSON. So we know roughly what it needs to look like. Next steps are to get that information.

Chromium has a whole API for this. I think we'll need one too, but maybe a bit simpler.

Suggested design at the moment:

  • A memory dumper is a function pointer, registered with systrace as an API call (systrace_memdump_register(fptr))
  • systrace_init spawns a thread which sleeps, and every N seconds, calls the registered memory dumpers.
  • Memory dumpers call systrace_memdump_report_byte_total("Subsystem", byteCount) where byteCount is the number of bytes used
  • At the end of the memdump thread's run, this is all passed in messages to traced for it to dump into JSON.

These of course will need thread-safety, then, but I think that there's not really any "nice" way to make this happen otherwise.

@special
Copy link

special commented Mar 31, 2017

I think the thread-safety concern is going to be a pretty significant problem, since these memory dumpers will often be iterating structs and containers to count up usage. There will be places that are impossible to count without adding a lot of mutexes to real-world code.

Is there a reason why different memory dumpers have to report at the same interval and moment? I think it would make more sense to allow the program to trigger them opportunistically.

Something like:

  • systrace_memdump_report_byte_total unequivocally generates a new report and can be called by the application at any time/interval
  • systrace_memdump_needs_report returns a bool indicating whether there has been a report from the given subsystem in the past N seconds. Should be used by the application to avoid doing expensive counting work more often than necessary
  • (if you're feeling fancy) systrace_memdump_report_byte_delta can be called in-place for significant alloc/free operations without calculating everything again

Then an application just needs to find a good place to call their memdump function, which could be part of a normal operation, or some kind of timer, or whatever works best. If it's in a hotter path that would generate too many memdump reports, the systrace_memdump_needs_report function can rate limit the work.

We can actually do the best of both: implement what I've written here along with an optional systrace_memdump_register. Spawn the memdump thread when the first one is registered.

There probably has to be a way to deregister a memdump function also.

@rburchell
Copy link
Member Author

I think the thread-safety concern is going to be a pretty significant problem, since these memory dumpers will often be iterating structs and containers to count up usage. There will be places that are impossible to count without adding a lot of mutexes to real-world code.

The way I imagined this working in practice is that the counter would be done as an atomic counter, on the main thread. The dumper would "just" be responsible for copying that value over at once. Nothing more.

Is there a reason why different memory dumpers have to report at the same interval and moment? I think it would make more sense to allow the program to trigger them opportunistically.

I need to look into this more, but I think they must all report at the same time for the dump to make "sense" according to the way trace-viewer interprets the information.

systrace_memdump_report_byte_total unequivocally generates a new report and can be called by the application at any time/interval

Allowing the application to trigger this (rather than explicitly threading it ourselves) is somewhat nicer, but means that we have to rely on the application developer to do something more than "systrace_init/deinit and copy the code over", which I have previously been trying to avoid. Hmm...

(if you're feeling fancy) systrace_memdump_report_byte_delta can be called in-place for significant alloc/free operations without calculating everything again

I was initially thinking along those very lines for the API, but keeping the atomic-permanent-value-that-we-just-copy that I was thinking here, that felt out of place...

@rburchell
Copy link
Member Author

Another thing to keep in mind: these counts will never be (and are not intended to be) 100% accurate. For that, you want a proper memory profiler like heaptrack or massif, I think. This is just a faster, lighter-weight alternative that gives you a finger-in-the-air type count. Think of it as being placed somewhere like hooked into QObject to give you accounting for what types of QObject instances were allocated at present. "allocCounter += sizeof(MyObject)" in a ctor/dtor type thing.

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

No branches or pull requests

2 participants