In statistics dump --summary, add back the targets section
#97004
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary: Test Plan: Reviewers: Subscribers: Tasks: Tags:
|
@llvm/pr-subscribers-lldb Author: None (royitaqi) ChangesChange#95075 accidentally removed the Updated the description for the options. VerificationManually verified the fix by running
The expected result is that 1 and 3 give the same sections, while 2 is missing the Appendix: Manual Test Output
|
cpiaseque
pushed a commit
to cpiaseque/llvm-project
that referenced
this pull request
Jul 3, 2024
…97004) # Change llvm#95075 accidentally removed the `targets` section from `statistics dump --summary`. Adding it back, by setting the default value to `true` in `StatisticsOptions::GetIncludeTargets()`. Updated the description for the options. Updated tests. # Verification Manually verified the fix by running `statist dump --summary` and comparing three versions of LLDB (in commit order): 1. Before llvm#95075 2. After llvm#95075 3. After this fix The expected result is that 1 and 3 give the same sections, while 2 is missing the `targets` section when in summary mode. The output (see Appendix) matches the expectation. # Appendix: Manual Test Output ## `statistics dump --summary` of 1 ``` (lldb) statistics dump --summary { "memory": { "strings": { "bytesTotal": 724992, "bytesUnused": 714547, "bytesUsed": 10445 } }, "targets": [ { "sourceMapDeduceCount": 0, "totalSharedLibraryEventHitCount": 0 } ], "totalDebugInfoByteSize": 597, "totalDebugInfoEnabled": 1, "totalDebugInfoIndexLoadedFromCache": 0, "totalDebugInfoIndexSavedToCache": 0, "totalDebugInfoIndexTime": 0.00070699999999999995, "totalDebugInfoParseTime": 2.5999999999999998e-05, "totalModuleCount": 1, "totalModuleCountHasDebugInfo": 1, "totalModuleCountWithIncompleteTypes": 0, "totalModuleCountWithVariableErrors": 0, "totalSymbolTableIndexTime": 0.000223, "totalSymbolTableParseTime": 0.00025799999999999998, "totalSymbolTableStripped": 0, "totalSymbolTablesLoadedFromCache": 0, "totalSymbolTablesSavedToCache": 0 } (lldb) ``` ## `statistics dump --summary` of 3 Should be the same as above. ``` (lldb) statistics dump --summary { "memory": { "strings": { "bytesTotal": 516096, "bytesUnused": 510353, "bytesUsed": 5743 } }, "targets": [ { "sourceMapDeduceCount": 0, "totalSharedLibraryEventHitCount": 0 } ], "totalDebugInfoByteSize": 597, "totalDebugInfoEnabled": 1, "totalDebugInfoIndexLoadedFromCache": 0, "totalDebugInfoIndexSavedToCache": 0, "totalDebugInfoIndexTime": 0.0022139999999999998, "totalDebugInfoParseTime": 0.00031700000000000001, "totalModuleCount": 1, "totalModuleCountHasDebugInfo": 1, "totalModuleCountWithIncompleteTypes": 0, "totalModuleCountWithVariableErrors": 0, "totalSymbolTableIndexTime": 0.0014499999999999999, "totalSymbolTableParseTime": 0.001848, "totalSymbolTableStripped": 0, "totalSymbolTablesLoadedFromCache": 0, "totalSymbolTablesSavedToCache": 0 } (lldb) ``` ## `statistics dump --summary` of 2 Should be missing the `targets` section. ``` (lldb) statistics dump --summary { "memory": { "strings": { "bytesTotal": 716800, "bytesUnused": 705887, "bytesUsed": 10913 } }, "totalDebugInfoByteSize": 597, "totalDebugInfoEnabled": 1, "totalDebugInfoIndexLoadedFromCache": 0, "totalDebugInfoIndexSavedToCache": 0, "totalDebugInfoIndexTime": 0.001374, "totalDebugInfoParseTime": 0.000174, "totalModuleCount": 1, "totalModuleCountHasDebugInfo": 1, "totalModuleCountWithIncompleteTypes": 0, "totalModuleCountWithVariableErrors": 0, "totalSymbolTableIndexTime": 0.00068300000000000001, "totalSymbolTableParseTime": 0.0010139999999999999, "totalSymbolTableStripped": 0, "totalSymbolTablesLoadedFromCache": 0, "totalSymbolTablesSavedToCache": 0 } (lldb) ``` Co-authored-by: royshi <royshi@meta.com>
kevinfrei
pushed a commit
to kevinfrei/llvm
that referenced
this pull request
Sep 25, 2024
Summary: This is to temporarily fix the test `TestVSCode`, before [PR llvm#97004](llvm#97004) can be upstreamed and pulled into `toolchain/llvm-sand/main`. The bug was that a previous PR ([llvm#95075](llvm#95075)) accidentally removed the `targets` section from `statistics dump --summary`, which this test relies on. The fix in [PR llvm#97004](llvm#97004) reverts that behavior, while the temporary fix in this diff just explicitly adds an option to make sure that the `targets` section is generated. This diff *can (but doesn't have to)* be reverted after [PR llvm#97004](llvm#97004) has been merged and pulled into `llvm-sand`. Test Plan: Manually tested after applying this diff on top of the release candidate that I'm working on. Reviewers: jeffreytan, #lldb_team Subscribers: gclayton, #lldb_team Differential Revision: https://phabricator.intern.facebook.com/D59139567
imyixinw
pushed a commit
to imyixinw/llvm-project
that referenced
this pull request
Sep 5, 2025
Summary: This is to temporarily fix the test `TestVSCode`, before [PR llvm#97004](llvm#97004) can be upstreamed and pulled into `toolchain/llvm-sand/main`. The bug was that a previous PR ([llvm#95075](llvm#95075)) accidentally removed the `targets` section from `statistics dump --summary`, which this test relies on. The fix in [PR llvm#97004](llvm#97004) reverts that behavior, while the temporary fix in this diff just explicitly adds an option to make sure that the `targets` section is generated. This diff *can (but doesn't have to)* be reverted after [PR llvm#97004](llvm#97004) has been merged and pulled into `llvm-sand`. Test Plan: Manually tested after applying this diff on top of the release candidate that I'm working on. Reviewers: jeffreytan, #lldb_team Subscribers: gclayton, #lldb_team Differential Revision: https://phabricator.intern.facebook.com/D59139567
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Change
#95075 accidentally removed the
targetssection fromstatistics dump --summary. Adding it back, by setting the default value totrueinStatisticsOptions::GetIncludeTargets().Updated the description for the options.
Updated tests.
Verification
Manually verified the fix by running
statist dump --summaryand comparing three versions of LLDB (in commit order):statistics dumpto control what sections are dumped #95075statistics dumpto control what sections are dumped #95075The expected result is that 1 and 3 give the same sections, while 2 is missing the
targetssection when in summary mode. The output (see Appendix) matches the expectation.Appendix: Manual Test Output
statistics dump --summaryof 1statistics dump --summaryof 3Should be the same as above.
statistics dump --summaryof 2Should be missing the
targetssection.