[Lldb-commits] [lldb] Add new Python API `SBCommandInterpreter::GetTranscript()` (PR #90703)

via lldb-commits lldb-commits at lists.llvm.org
Tue May 7 11:28:05 PDT 2024


royitaqi wrote:

Hi @jimingham ,

> Sorry to come in a little late on this

No worries. Thanks for chiming in.

> I think we need a setting to turn this off as well. If you aren't planning to use the transcript, you shouldn't have to pay the cost for it.

Could you elaborate about why this is necessary? If we really need this, can I add the setting in a follow-up PR?

--

FWIW, to help make the conversation faster (reduce rounds of communication), I will write down my gut feeling below. Please kindly correct me if I'm wrong.

**First**, why I think the added cost is negligible:
1. The baseline is that there is already a `m_transcript_stream`, which doesn't have any settings to turn off. This PR just doubles that cost (plus some string split operations). It doesn't add huge cost on top of this baseline.
2. Maintaining transcript is a per-command operation, and commands are relatively infrequently invoked. Here I assume the common use cases run tens or at most hundreds of commands in a debug session. The above cost times 10x ~ 100x isn't much.
3. The cost of the commands themselves (and the process) are much higher than that of the transcript. E.g. resolving breakpoints and printing variables all need to do actual work, rather than just recording info.

**Secondly**, if we are going to add settings, there needs to be product design considerations _based on user usage/feedback_, so we probably need more time to gather data points after shipping this feature.
1. Prematurely add settings (which we cannot remove, I assume) can lead us down to a more difficult road.
2. I think it will be the norm that we record transcripts, like bash/zsh records command histories, so by default this should be on. In fact it is already, by `m_transcript_stream`.
3. Besides just a binary on/off, settings can also allow more granular control. E.g. verbosity; only log transcript for a subset of commands; limit the memory footprint by using a cyclic buffer; etc. If we add these settings, they will cause the on/off setting to be redundant and needs to be removed.
4. (As mentioned in "Secondly") The above are all possibilities. We need user usage/feedback to guide the design of these settings. So I think we should ship this PR first then gather data points.


https://github.com/llvm/llvm-project/pull/90703


More information about the lldb-commits mailing list