[lldb-vscode] Make descriptive summaries and raw child for synthetics configurable #65687
Conversation
walter-erquinigo
force-pushed
the
walter/settings
branch
from
5b1250c to
fc0700e
Compare
lldb/tools/lldb-vscode/VSCode.cpp
Outdated
| @@ -40,7 +40,8 @@ VSCode::VSCode() | |||
| {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift}, | |||
| {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}), | |||
| focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false), | |||
| stop_at_entry(false), is_attach(false), | |||
| stop_at_entry(false), is_attach(false), show_descriptive_summaries(false), | |||
| show_raw_child_for_synthetics(false), | |||
There was a problem hiding this comment.
Shouldn't show_raw_child_for_synthetics be true by default?
There was a problem hiding this comment.
No, we want these off by default. I don't want to see a "[raw]" entry at the end of all synthetic variables each time I debug. Also the descriptive summaries are way too expensive to enable as they will cause all types to always be completed. Say you have 100 variables in your variable view, if you don't turn them down in the variable view, we never need to complete the types for these entries, but with this change enabled, all variable types (and if each type is a class, all subclasses, and all ivars from the current and subclasses will be completed) will always be completed even if they aren't turned down. This was the reason we originally made the "SBValue::MightHaveChildren()" (it doesn't complete a type, it just says "if you are a struct, then you might have children" and then we can display the disclosure triange in the UI without completing the type) as objective C debugging variables were always class pointers that didn't need to be completed. We saw a huge performance gain when we didn't complete a type and displaying variables in Xcode many years ago, so we know this is an issue.
There was a problem hiding this comment.
Thanks for the context Greg. The commit description and the rawChildForSynthetics property both say the default is true, so that needs to be updated. My comment here was strictly referring to the boolean to make sure it was consistent with the other two, but it seems like that's not what we want.
lldb/tools/lldb-vscode/VSCode.cpp
Outdated
| @@ -40,7 +40,8 @@ VSCode::VSCode() | |||
| {"swift_catch", "Swift Catch", lldb::eLanguageTypeSwift}, | |||
| {"swift_throw", "Swift Throw", lldb::eLanguageTypeSwift}}), | |||
| focus_tid(LLDB_INVALID_THREAD_ID), sent_terminated_event(false), | |||
| stop_at_entry(false), is_attach(false), | |||
| stop_at_entry(false), is_attach(false), show_descriptive_summaries(false), | |||
| show_raw_child_for_synthetics(false), | |||
There was a problem hiding this comment.
No, we want these off by default. I don't want to see a "[raw]" entry at the end of all synthetic variables each time I debug. Also the descriptive summaries are way too expensive to enable as they will cause all types to always be completed. Say you have 100 variables in your variable view, if you don't turn them down in the variable view, we never need to complete the types for these entries, but with this change enabled, all variable types (and if each type is a class, all subclasses, and all ivars from the current and subclasses will be completed) will always be completed even if they aren't turned down. This was the reason we originally made the "SBValue::MightHaveChildren()" (it doesn't complete a type, it just says "if you are a struct, then you might have children" and then we can display the disclosure triange in the UI without completing the type) as objective C debugging variables were always class pointers that didn't need to be completed. We saw a huge performance gain when we didn't complete a type and displaying variables in Xcode many years ago, so we know this is an issue.
walter-erquinigo
force-pushed
the
walter/settings
branch
from
fc0700e to
4d563d5
Compare
|
@clayborg , do you get notifications when I update the PR? :) I'm still getting used to github for llvm. |
|
Note that the formatting check failed! It shouldn't be ignored :) |
… configurable "descriptive summaries" should only be used for small to medium binaries because of the performance penalty the cause when completing types. I'm defaulting it to false. Besides that, the "raw child" for synthetics should be optional as well. I'm defaulting it to false as well. Both options can be set via a launch or attach config, following the pattern of most settings. javascript extension wrappers can set these settings on their own as well.
walter-erquinigo
force-pushed
the
walter/settings
branch
from
4d563d5 to
6415738
Compare
… configurable (llvm#65687) "descriptive summaries" should only be used for small to medium binaries because of the performance penalty the cause when completing types. I'm defaulting it to false. Besides that, the "raw child" for synthetics should be optional as well. I'm defaulting it to false. Both options can be set via a launch or attach config, following the pattern of most settings. javascript extension wrappers can set these settings on their own as well.
… configurable (llvm#65687) "descriptive summaries" should only be used for small to medium binaries because of the performance penalty the cause when completing types. I'm defaulting it to false. Besides that, the "raw child" for synthetics should be optional as well. I'm defaulting it to false. Both options can be set via a launch or attach config, following the pattern of most settings. javascript extension wrappers can set these settings on their own as well.
… configurable (llvm#65687) "descriptive summaries" should only be used for small to medium binaries because of the performance penalty the cause when completing types. I'm defaulting it to false. Besides that, the "raw child" for synthetics should be optional as well. I'm defaulting it to false. Both options can be set via a launch or attach config, following the pattern of most settings. javascript extension wrappers can set these settings on their own as well. (cherry picked from commit a2a9918)
"descriptive summaries" should only be used for small to medium binaries because of the performance penalty the cause when completing types. I'm defaulting it to false.
Besides that, the "raw child" for synthetics should be optional as well. I'm defaulting it to false.
Both options can be set via a launch or attach config, following the pattern of most settings. javascript extension wrappers can set these settings on their own as well.