Track memory allocations and writes for debug info #14056
Conversation
|
Looks very interesting! Will look at this in detail next week. |
|
There was a performance impact before, so I made some changes - it should be gone now. Deferring might be bad in a way, making any impact more lumpy - but it's pretty fast anyway, now. I was also considering moving it to a thread, but the -[Unknown] |
unknownbrackets
force-pushed
the
debugger-mem
branch
3 times, most recently
from
0c353e5
to
4353e39
Compare
This is useful info.
It's not so slow, but let's defer (could even use a thread.)
This is simpler, we're always comparing the end anyway.
This is a very common case, so helps skip ahead.
Does not visualize yet, just implements the selection interface.
We start with a large unmarked region which we break up. Ignore this.
This also grabs tag info, but doesn't display it yet.
Oops, was crashing before since it's outside the index.
And include the start and end of the range.
Oops, wasn't clearing them so was eventually maxing out fragmentation.
unknownbrackets
force-pushed
the
debugger-mem
branch
from
4353e39
to
07de2e5
Compare
|
Sorry this has taken so long, I'm gonna review it soon. |
Core/Debugger/Breakpoints.cpp
Outdated
| @@ -40,14 +40,15 @@ static std::mutex memCheckMutex_; | |||
| std::vector<MemCheck> CBreakPoints::memChecks_; | |||
| std::vector<MemCheck *> CBreakPoints::cleanupMemChecks_; | |||
|
|
|||
| void MemCheck::Log(u32 addr, bool write, int size, u32 pc) { | |||
| void MemCheck::Log(u32 addr, bool write, int size, u32 pc, const std::string &reason) { | |||
There was a problem hiding this comment.
Since reason is used as .c_str() only in this function, I'd prefer a plain char pointer here, to avoid a memory allocation. (string_view would be ideal for things like this but is a little too new and not smoothly supported by sprintf)
There are some similar cases in other parts of this PR.
There was a problem hiding this comment.
This parameter can be changed - but for other parts:
The framebuf and texture ones are dynamic strings, and almost all of these are gonna be 20 bytes or less, so they shouldn't be separate allocations anyway. I didn't want to have to worry about allocation lifetimes for the dynamic ones, and I definitely don't want to require that these all be static.
Also: these are included in save states for probably obvious reasons - which means finding the static string won't be easy anyway. And no way this'll be useful if as soon as I step back to a previous save state, all the tags are gone. So they'll be std::strings in the end anyway.
-[Unknown]
There was a problem hiding this comment.
Hm, didn't realize how many of these were dynamic. I'm ok with it now, thanks for changing in memchecks at least.
This keeps track of memory allocation, suballocations, and writes for debug info.
As of writing, there's not yet any UI or debugger API to access this info, it's just the internals so far.
This is handled by notating reasons on memory writes/allocations, which are also exposed in memory breakpoint hits. This does mean a lot of spread out changes, but it should give a vague picture of how memory is being used.
I decided to track in "layers" so we can have allocation info + write info. All of this is "latest available", and when an allocation is deallocated, the previous allocation tag is kept. It's also tracking the PC when the activity happened.
I was considering, if you enable something, maybe CPU writes could even track into this by using a fixed size buffer of
{ addr, pc }or something. Once the buffer is full, call into C++ to flush. It'd be slower, but maybe bearable. That said I've also been considering options for reverse debugging that could be more efficient anyway.One thing I haven't thought about is user/api tagging. This might add another "layer".
Feedback appreciated - tried to keep any additional effort surgical. Also, because the Net code is in perhaps more flux I didn't add tags in there yet to avoid conflicts (I'm expecting this to get merged sometime after v1.11.0.)
Lastly, I've tentatively decided to include in save states. Two reasons: first, save states are frequently used during debugging and it'd be a huge loss otherwise; second, if someone sends a save state of some bug or problem it could be very helpful. I didn't check or really estimate how much extra space it adds, though it's probably not crazy.
-[Unknown]