[SAL] Introduce support for multiple memory types #6
Conversation
stettberger
force-pushed
the
feature/memory-types
branch
2 times, most recently
from
4060c44 to
3d74a26
Compare
stettberger
force-pushed
the
generic-experiment-improvements
branch
from
a0a1970 to
9a5b560
Compare
stettberger
force-pushed
the
feature/memory-types
branch
from
3d74a26 to
618993b
Compare
stettberger
changed the base branch from
generic-experiment-improvements
to
master
stettberger
force-pushed
the
feature/memory-types
branch
from
618993b to
c7e4c74
Compare
Rothu
left a comment
Rothu
left a comment
There was a problem hiding this comment.
There are potential memory bugs in the commented locations.
| BochsController::~BochsController() | ||
| { | ||
| delete m_Mem; | ||
| delete &getMemoryManager(); |
There was a problem hiding this comment.
At this point only the MemoryManager for MEMTYPE_RAM would be deleted, since this is the default for getMemoryManager().
| it = m_CPUs.erase(it); | ||
| } | ||
| delete m_Mem; | ||
| delete &getMemoryManager(); |
There was a problem hiding this comment.
At this point only the MemoryManager for MEMTYPE_RAM would be deleted, since this is the default for getMemoryManager().
| delete *it; | ||
| it = m_CPUs.erase(it); | ||
| } | ||
| delete &getMemoryManager(); |
There was a problem hiding this comment.
At this point only the MemoryManager for MEMTYPE_RAM would be deleted, since this is the default for getMemoryManager().
There was a problem hiding this comment.
This is fine for Panda, since there is only one kind of memor manager.
| { | ||
| delete m_Regs; | ||
| delete m_Mem; | ||
| delete &getMemoryManager(); |
There was a problem hiding this comment.
At this point only the MemoryManager for MEMTYPE_RAM would be deleted, since this is the default for getMemoryManager().
There was a problem hiding this comment.
This is fine for QEMU, since there is only one kind of memor manager.
|
The memory-leak issues aside (I could live with these for the time being), I observed this change to incur 6-18% slowdown for the generic-tracing experiment. I didn't test generic-experiment, but would expect a similar effect on FI-campaign runtime. Is it really necessary to call getMemoryManager(MEMTYPE_RAM) (which does an std::map lookup) and retrieve the read/written value for every memory access in FastWatchpoints.ah? |
I could not trace back a performance degredation back to this function call. I also tried to insert a special case for MEMTYPE_RAM to solve this problem, which circumvents the usage of the std::map() lookup(). However, I could not identify any difference in speed. When looking at the Flamegraph (see flamegraph.zip) the getMemoryManager() has an overall run-time influence of 0.2% (If i use the search correctly) |
Many architectures have different types of memory (flash, RAM, EEPROM, etc), which is currently not well supported in the FAIL* toolchain. Therefore, this change introduces memory types. A memory type is a simple tag that is attached to memory events and memory listeners. Furthermore, the generic-tracing experiment records memory types and dump-trace displays them. Currently, they are not yet used in importing. However, with SAIL integration, we require this kind of disambiguation between different memory types. Furthermore, MemoryAccessEvents now carry the memory area that was accessed. This change should not break previous experiments and/or results. Co-authored-by: Malte Bargholz <malte@screenri.de>
stettberger
force-pushed
the
feature/memory-types
branch
from
c7e4c74 to
168203b
Compare
Many architectures have different types of memory (flash, RAM, EEPROM,
etc), which is currently not well supported in the FAIL* toolchain.
Therefore, this change introduces memory types.
A memory type is a simple tag that is attached to memory events and
memory listeners. Furthermore, the generic-tracing experiment records
memory types and dump-trace displays them. Currently, they are not yet
used in importing. However, with SAIL integration, we require this
kind of disambiguation between different memory types.
This change should not break previous experiments and/or results.