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

i#7113 decode cache: move module read into raw2trace_shared #7124

Merged
merged 13 commits into from
Dec 15, 2024

Conversation

abhinav92003
Copy link
Contributor

@abhinav92003 abhinav92003 commented Dec 11, 2024

Moves the module read functionality into raw2trace_shared.

Refactors opcode_mix and view to use the new function instead of creating a raw2trace_directory_t.

This is in preparation for further changes that include the module read logic in a new library (drmemtrace_decode_cache) that provides an instr decode cache (decode_cache_t). To avoid having to pull in the whole drmemtrace_raw2trace implementation into drmemtrace_decode_cache, which will end up including it in the invariant checker and everything that depends on, we separate out the module read logic into raw2trace_shared which is intended for such cases.

While the raw2trace and raw2trace_shared implementation usually is made to deal with only file streams, we make an exception here for read_module_file() which uses DR file APIs instead. This is likely fine because this function is not intended for use with code that wants file streams, and it is better than other alternatives: moving raw2trace-using functionality out of raw2trace_directory_t so we can use raw2trace_directory_t in decode_cache_t (this would still add zlib/lz4 etc unnecessary file libs to drmemtrace_decode_cache), create yet another raw2trace_shared_non_stream.cpp for shared code that uses DR file APIs, or duplicating the read_module_file impl in decode_cache_t.

This also removes raw2trace_directory_t::initialize_module_file() as that functionality is now provided by read_module_file in raw2trace_shared.

Issue: #7113

This is in preparation for further changes that include the module mapper logic in a new library (drmemtrace_instr_decode_cache) that provides an instr decode cache (instr_decode_cache_t). To avoid having to pull in the whole drmemtrace_raw2trace implementation into drmemtrace_instr_decode_cache, which will end up including it in the invariant checker and everything that depends on, we separate out the module mapper into raw2trace_shared which is intended for such cases.

Issue: #7113
@abhinav92003 abhinav92003 marked this pull request as draft December 11, 2024 17:59
@abhinav92003
Copy link
Contributor Author

Discovered that there are complexities related to the lifecycle of the modfile_bytes. It must be available throughout and not just at init time:

// We reference directory.modfile_bytes throughout operation, so its lifetime
.

This will be solved when the module mapping logic is moved to instr_decode_cache_t. Maybe better to just do that directly.

Converted to draft for now.

@abhinav92003 abhinav92003 marked this pull request as ready for review December 12, 2024 16:20
@abhinav92003
Copy link
Contributor Author

Discovered that there are complexities related to the lifecycle of the modfile_bytes. It must be available throughout and not just at init time:

// We reference directory.modfile_bytes throughout operation, so its lifetime

.
This will be solved when the module mapping logic is moved to instr_decode_cache_t. Maybe better to just do that directly.

Converted to draft for now.

#7114 is already becoming too large, so better to do this move separately. I added a separate modfile_bytes_ for now.

clients/drcachesim/CMakeLists.txt Show resolved Hide resolved
clients/drcachesim/CMakeLists.txt Outdated Show resolved Hide resolved
clients/drcachesim/tools/view.cpp Show resolved Hide resolved
@abhinav92003 abhinav92003 changed the title i#7113 instr decode cache: move module read into raw2trace_shared i#7113 decode cache: move module read into raw2trace_shared Dec 15, 2024
@abhinav92003 abhinav92003 merged commit 73aae26 into master Dec 15, 2024
17 checks passed
@abhinav92003 abhinav92003 deleted the i7113-move-read-module branch December 15, 2024 07:20
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.

2 participants