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

Add PID information to -LL_miss_file output #6896

Merged

Conversation

jin11109
Copy link
Contributor

When using simulator parameters -LL_miss_file option, I noticed that the output was insufficient for detailed analysis, especially when distinguishing cache misses across processes. This limitation becomes more apparent in environments where multiple processes may experience cache misses at similar virtual memory addresses, potentially leading to ambiguous outputs.

So, I have updated the dump_miss function within caching_device_stats.cpp to include the PID in the output. This small yet impactful change ensures that each cache miss log entry is prefixed with the PID of the process, thereby enabling users to better correlate the cache misses with specific processes.

Copy link
Contributor

@derekbruening derekbruening left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for contributing. It looks like a few tweaks are needed: see the detailed responses.

clients/drcachesim/simulator/caching_device_stats.cpp Outdated Show resolved Hide resolved
clients/drcachesim/simulator/caching_device_stats.cpp Outdated Show resolved Hide resolved
clients/drcachesim/simulator/caching_device_stats.cpp Outdated Show resolved Hide resolved
clients/drcachesim/simulator/caching_device_stats.cpp Outdated Show resolved Hide resolved
@jin11109 jin11109 marked this pull request as draft July 23, 2024 17:26
@jin11109 jin11109 marked this pull request as ready for review July 24, 2024 15:05
@derekbruening
Copy link
Contributor

(Please reply to each review comment and mark resolved if resolved and then click re-request review when ready.)

@jin11109 jin11109 marked this pull request as draft July 25, 2024 10:43
@jin11109 jin11109 marked this pull request as ready for review July 25, 2024 11:14
@derekbruening
Copy link
Contributor

(Click the circular arrow when ready for re-review; xref docs https://dynamorio.org/page_code_reviews.html#autotoc_md118)

api/docs/release.dox Outdated Show resolved Hide resolved
clients/drcachesim/common/options.cpp Show resolved Hide resolved
@derekbruening derekbruening merged commit 7d39db4 into DynamoRIO:master Aug 1, 2024
17 checks passed
@derekbruening
Copy link
Contributor

Merged. Thank you for contributing!

@jin11109 jin11109 deleted the feature-add-pid-to-LL_miss_file branch August 2, 2024 03:34
@joshua-warburton
Copy link
Collaborator

It seems this patch causes compilation errors on more recent (17) versions of clang:

/.../dynamorio/clients/drcachesim/simulator/caching_device_stats.cpp:173:56: error: format '%lld' expects argument of type 'long long int', but argument 3 has type 'dynamorio::drmemtrace::memref_pid_t {aka long int}' [-Werror=format=]

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.

3 participants