clayborg updated this revision to Diff 327339. clayborg added a comment. Added a "uint64_t progress_id;" as a member variable of the lldb_private::Progress class to help users track individual progress dialogs without conflict. Prior to this the message was assumed to be unique.
Added code to ensure that we only report one update there "completed == total". This allows clients that install the callbacks to ensure they will only receive one completion event. Added code the ensure we don't increment past the total amount and also can't run into unsigned overflow conditions that could have resulted in sending an update where the new "completed" amount was less than before. Added header doc to explain the usage of the progress class. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D97739/new/ https://reviews.llvm.org/D97739 Files: lldb/include/lldb/API/SBDebugger.h lldb/include/lldb/Core/Progress.h lldb/include/lldb/lldb-types.h lldb/source/API/SBDebugger.cpp lldb/source/Core/CMakeLists.txt lldb/source/Core/Progress.cpp lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp lldb/tools/lldb-vscode/VSCode.cpp lldb/tools/lldb-vscode/VSCode.h lldb/tools/lldb-vscode/lldb-vscode.cpp
Index: lldb/tools/lldb-vscode/lldb-vscode.cpp =================================================================== --- lldb/tools/lldb-vscode/lldb-vscode.cpp +++ lldb/tools/lldb-vscode/lldb-vscode.cpp @@ -1448,6 +1448,8 @@ body.try_emplace("supportsDelayedStackTraceLoading", true); // The debug adapter supports the 'loadedSources' request. body.try_emplace("supportsLoadedSourcesRequest", false); + // The debug adapter supports sending progress reporting events. + body.try_emplace("supportsProgressReporting", true); response.try_emplace("body", std::move(body)); g_vsc.SendJSON(llvm::json::Value(std::move(response))); @@ -3036,6 +3038,11 @@ #endif } +void LLDBProgressCallback(uint64_t progress_id, const char *message, + uint64_t completed, uint64_t total, void *baton) { + g_vsc.SendProgressEvent(progress_id, message, completed, total); +} + int main(int argc, char *argv[]) { llvm::SmallString<256> program_path(argv[0]); llvm::sys::fs::make_absolute(program_path); @@ -3065,6 +3072,7 @@ // Initialize LLDB first before we do anything. lldb::SBDebugger::Initialize(); + lldb::SBDebugger::SetProgressCallback(LLDBProgressCallback, nullptr); RegisterRequestCallbacks(); Index: lldb/tools/lldb-vscode/VSCode.h =================================================================== --- lldb/tools/lldb-vscode/VSCode.h +++ lldb/tools/lldb-vscode/VSCode.h @@ -132,6 +132,9 @@ void SendOutput(OutputType o, const llvm::StringRef output); + void SendProgressEvent(uint64_t progress_id, const char *message, + uint64_t completed, uint64_t total); + void __attribute__((format(printf, 3, 4))) SendFormattedOutput(OutputType o, const char *format, ...); Index: lldb/tools/lldb-vscode/VSCode.cpp =================================================================== --- lldb/tools/lldb-vscode/VSCode.cpp +++ lldb/tools/lldb-vscode/VSCode.cpp @@ -225,6 +225,142 @@ SendJSON(llvm::json::Value(std::move(event))); } +// interface ProgressStartEvent extends Event { +// event: 'progressStart'; +// +// body: { +// /** +// * An ID that must be used in subsequent 'progressUpdate' and +// 'progressEnd' +// * events to make them refer to the same progress reporting. +// * IDs must be unique within a debug session. +// */ +// progressId: string; +// +// /** +// * Mandatory (short) title of the progress reporting. Shown in the UI to +// * describe the long running operation. +// */ +// title: string; +// +// /** +// * The request ID that this progress report is related to. If specified a +// * debug adapter is expected to emit +// * progress events for the long running request until the request has +// been +// * either completed or cancelled. +// * If the request ID is omitted, the progress report is assumed to be +// * related to some general activity of the debug adapter. +// */ +// requestId?: number; +// +// /** +// * If true, the request that reports progress may be canceled with a +// * 'cancel' request. +// * So this property basically controls whether the client should use UX +// that +// * supports cancellation. +// * Clients that don't support cancellation are allowed to ignore the +// * setting. +// */ +// cancellable?: boolean; +// +// /** +// * Optional, more detailed progress message. +// */ +// message?: string; +// +// /** +// * Optional progress percentage to display (value range: 0 to 100). If +// * omitted no percentage will be shown. +// */ +// percentage?: number; +// }; +// } +// +// interface ProgressUpdateEvent extends Event { +// event: 'progressUpdate'; +// +// body: { +// /** +// * The ID that was introduced in the initial 'progressStart' event. +// */ +// progressId: string; +// +// /** +// * Optional, more detailed progress message. If omitted, the previous +// * message (if any) is used. +// */ +// message?: string; +// +// /** +// * Optional progress percentage to display (value range: 0 to 100). If +// * omitted no percentage will be shown. +// */ +// percentage?: number; +// }; +// } +// +// interface ProgressEndEvent extends Event { +// event: 'progressEnd'; +// +// body: { +// /** +// * The ID that was introduced in the initial 'ProgressStartEvent'. +// */ +// progressId: string; +// +// /** +// * Optional, more detailed progress message. If omitted, the previous +// * message (if any) is used. +// */ +// message?: string; +// }; +// } + +void VSCode::SendProgressEvent(uint64_t progress_id, const char *message, + uint64_t completed, uint64_t total) { + enum ProgressEventType { + progressInvalid, + progressStart, + progressUpdate, + progressEnd + }; + const char *event_name = nullptr; + ProgressEventType event_type = progressInvalid; + if (completed == 0) { + event_type = progressStart; + event_name = "progressStart"; + } else if (completed == total) { + event_type = progressEnd; + event_name = "progressEnd"; + } else if (completed < total) { + event_type = progressUpdate; + event_name = "progressUpdate"; + } + if (event_type == progressInvalid) + return; + + llvm::json::Object event(CreateEventObject(event_name)); + llvm::json::Object body; + std::string progress_id_str; + llvm::raw_string_ostream progress_id_strm(progress_id_str); + progress_id_strm << progress_id; + progress_id_strm.flush(); + body.try_emplace("progressId", progress_id_str); + if (event_type == progressStart) { + EmplaceSafeString(body, "title", message); + body.try_emplace("cancellable", false); + } + + if (0 < total && total < UINT64_MAX) { + uint32_t percentage = (uint32_t)(((float)completed / (float)total) * 100.0); + body.try_emplace("percentage", percentage); + } + event.try_emplace("body", std::move(body)); + SendJSON(llvm::json::Value(std::move(event))); +} + void __attribute__((format(printf, 3, 4))) VSCode::SendFormattedOutput(OutputType o, const char *format, ...) { char buffer[1024]; Index: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp =================================================================== --- lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp +++ lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp @@ -16,6 +16,7 @@ #include "lldb/Core/ModuleList.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Core/Progress.h" #include "lldb/Core/Section.h" #include "lldb/Core/StreamFile.h" #include "lldb/Core/Value.h" @@ -466,6 +467,10 @@ void SymbolFileDWARF::InitializeObject() { Log *log = LogChannelDWARF::GetLogIfAll(DWARF_LOG_DEBUG_INFO); + Progress progress( + UINT64_MAX, "Indexing DWARF for %s", + GetObjectFile()->GetFileSpec().GetFilename().AsCString("<Unknown>")); + if (!GetGlobalPluginProperties()->IgnoreFileIndexes()) { DWARFDataExtractor apple_names, apple_namespaces, apple_types, apple_objc; LoadSectionData(eSectionTypeDWARFAppleNames, apple_names); Index: lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp =================================================================== --- lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp +++ lldb/source/Plugins/ObjectFile/Mach-O/ObjectFileMachO.cpp @@ -17,6 +17,7 @@ #include "lldb/Core/Module.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Core/Progress.h" #include "lldb/Core/Section.h" #include "lldb/Core/StreamFile.h" #include "lldb/Host/Host.h" @@ -1893,9 +1894,9 @@ filename = first_section_sp->GetObjectFile()->GetFileSpec().GetPath(); Host::SystemLog(Host::eSystemLogError, - "error: unable to find section %d for a symbol in %s, corrupt file?\n", - n_sect, - filename.c_str()); + "error: unable to find section %d for a symbol in " + "%s, corrupt file?\n", + n_sect, filename.c_str()); } } if (m_section_infos[n_sect].vm_range.Contains(file_addr)) { @@ -2167,6 +2168,9 @@ if (!module_sp) return 0; + Progress progress(UINT64_MAX, "Parsing symbol table for %s", + m_file.GetFilename().AsCString("<Unknown>")); + struct symtab_command symtab_load_command = {0, 0, 0, 0, 0, 0}; struct linkedit_data_command function_starts_load_command = {0, 0, 0, 0}; struct dyld_info_command dyld_info = {0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}; Index: lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp =================================================================== --- lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp +++ lldb/source/Plugins/ObjectFile/ELF/ObjectFileELF.cpp @@ -16,6 +16,7 @@ #include "lldb/Core/Module.h" #include "lldb/Core/ModuleSpec.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Core/Progress.h" #include "lldb/Core/Section.h" #include "lldb/Host/FileSystem.h" #include "lldb/Host/LZMA.h" @@ -1861,7 +1862,7 @@ // unified section list. if (GetType() != eTypeDebugInfo) unified_section_list = *m_sections_up; - + // If there's a .gnu_debugdata section, we'll try to read the .symtab that's // embedded in there and replace the one in the original object file (if any). // If there's none in the orignal object file, we add it to it. @@ -1879,7 +1880,7 @@ unified_section_list.AddSection(symtab_section_sp); } } - } + } } std::shared_ptr<ObjectFileELF> ObjectFileELF::GetGnuDebugDataObjectFile() { @@ -1923,7 +1924,7 @@ ArchSpec spec = m_gnu_debug_data_object_file->GetArchitecture(); if (spec && m_gnu_debug_data_object_file->SetModulesArchitecture(spec)) return m_gnu_debug_data_object_file; - + return nullptr; } @@ -2707,6 +2708,9 @@ if (!module_sp) return nullptr; + Progress progress(UINT64_MAX, "Parsing symbol table for %s", + m_file.GetFilename().AsCString("<Unknown>")); + // We always want to use the main object file so we (hopefully) only have one // cached copy of our symtab, dynamic sections, etc. ObjectFile *module_obj_file = module_sp->GetObjectFile(); Index: lldb/source/Core/Progress.cpp =================================================================== --- /dev/null +++ lldb/source/Core/Progress.cpp @@ -0,0 +1,64 @@ +//===-- Progress.cpp ------------------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "lldb/Core/Progress.h" + +#include "lldb/Utility/StreamString.h" + +using namespace lldb; +using namespace lldb_private; + +lldb::ProgressCallback Progress::g_callback = nullptr; +void *Progress::g_callback_baton = nullptr; +uint64_t Progress::g_progress_id = 0; + +Progress::Progress(uint64_t total, const char *format, ...) + : m_message(), m_completed(0), m_total(total), m_id(++g_progress_id) { + assert(total > 0); + StreamString message; + va_list args; + va_start(args, format); + message.PrintfVarArg(format, args); + va_end(args); + m_message = ConstString(message.GetString()); + ReportProgress(); +} + +Progress::~Progress() { + // Make sure to always report progress completed when this object is + // destructed so it indicates the progress dialog/activity should go away. + m_completed = m_total; + ReportProgress(); +} + +void Progress::Increment(uint64_t amount) { + if (amount > 0) { + // Watch out for unsigned overflow and make sure we don't increment too + // much and exceed m_total. + if (amount > (m_total - m_completed)) + m_completed = m_total; + else + m_completed += amount; + ReportProgress(); + } +} + +void Progress::SetProgressCallback(ProgressCallback callback, void *baton) { + g_callback = callback; + g_callback_baton = baton; +} + +void Progress::ReportProgress() { + if (g_callback && !m_complete) { + // Make sure we only send one notification that indicates the progress is + // complete. + m_complete = m_completed == m_total; + g_callback(m_id, m_message.GetCString(), m_completed, m_total, + g_callback_baton); + } +} Index: lldb/source/Core/CMakeLists.txt =================================================================== --- lldb/source/Core/CMakeLists.txt +++ lldb/source/Core/CMakeLists.txt @@ -44,6 +44,7 @@ ModuleList.cpp Opcode.cpp PluginManager.cpp + Progress.cpp RichManglingContext.cpp SearchFilter.cpp Section.cpp Index: lldb/source/API/SBDebugger.cpp =================================================================== --- lldb/source/API/SBDebugger.cpp +++ lldb/source/API/SBDebugger.cpp @@ -38,6 +38,7 @@ #include "lldb/Core/Debugger.h" #include "lldb/Core/PluginManager.h" +#include "lldb/Core/Progress.h" #include "lldb/Core/StreamFile.h" #include "lldb/Core/StructuredDataImpl.h" #include "lldb/DataFormatters/DataVisualization.h" @@ -251,6 +252,10 @@ ModuleList::RemoveOrphanSharedModules(mandatory); } +void SBDebugger::SetProgressCallback(ProgressCallback callback, void *baton) { + Progress::SetProgressCallback(callback, baton); +} + bool SBDebugger::IsValid() const { LLDB_RECORD_METHOD_CONST_NO_ARGS(bool, SBDebugger, IsValid); return this->operator bool(); @@ -808,23 +813,25 @@ // The version of CreateTarget that takes an ArchSpec won't accept an // empty ArchSpec, so when the arch hasn't been specified, we need to // call the target triple version. - error = m_opaque_sp->GetTargetList().CreateTarget(*m_opaque_sp, filename, - arch_cstr, eLoadDependentsYes, nullptr, target_sp); + error = m_opaque_sp->GetTargetList().CreateTarget( + *m_opaque_sp, filename, arch_cstr, eLoadDependentsYes, nullptr, + target_sp); } else { PlatformSP platform_sp = m_opaque_sp->GetPlatformList() .GetSelectedPlatform(); - ArchSpec arch = Platform::GetAugmentedArchSpec(platform_sp.get(), - arch_cstr); + ArchSpec arch = + Platform::GetAugmentedArchSpec(platform_sp.get(), arch_cstr); if (arch.IsValid()) - error = m_opaque_sp->GetTargetList().CreateTarget(*m_opaque_sp, filename, - arch, eLoadDependentsYes, platform_sp, target_sp); + error = m_opaque_sp->GetTargetList().CreateTarget( + *m_opaque_sp, filename, arch, eLoadDependentsYes, platform_sp, + target_sp); else error.SetErrorStringWithFormat("invalid arch_cstr: %s", arch_cstr); } if (error.Success()) sb_target.SetSP(target_sp); } - + LLDB_LOGF(log, "SBDebugger(%p)::CreateTargetWithFileAndArch (filename=\"%s\", " "arch=%s) => SBTarget(%p)", Index: lldb/include/lldb/lldb-types.h =================================================================== --- lldb/include/lldb/lldb-types.h +++ lldb/include/lldb/lldb-types.h @@ -73,6 +73,9 @@ void *baton, const char **argv, lldb_private::CommandReturnObject &result); typedef bool (*ExpressionCancelCallback)(ExpressionEvaluationPhase phase, void *baton); +typedef void (*ProgressCallback)(uint64_t progress_id, const char *message, + uint64_t completed, uint64_t total, + void *baton); } // namespace lldb #define LLDB_INVALID_PROCESS ((lldb::process_t)-1) Index: lldb/include/lldb/Core/Progress.h =================================================================== --- /dev/null +++ lldb/include/lldb/Core/Progress.h @@ -0,0 +1,115 @@ +//===-- Progress.h ----------------------------------------------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLDB_CORE_PROGRESS_H +#define LLDB_CORE_PROGRESS_H + +#include "lldb/Utility/ConstString.h" +#include "lldb/lldb-types.h" +#include <atomic> + +namespace lldb_private { + +/// A Progress indicator helper class. +/// +/// Any potentially long running sections of code in LLDB should report +/// progress so that clients are aware of delays that might appear during +/// debugging. Delays commonly include indexing debug information, parsing +/// symbol tables for object files, downloading symbols from remote +/// repositories, and many more things. +/// +/// The Progress class helps make sure that progress is correctly reported +/// and will always send an initial progress update, updates when +/// Progress::Increment() is called, and also will make sure that a progress +/// completed update is reported even if the user doesn't explicitly cause one +/// to be sent. +/// +/// The progress is reported via a callback whose type is ProgressCallback: +/// +/// typedef void (*ProgressCallback)(uint64_t progress_id, +/// const char *message, +/// uint64_t completed, +/// uint64_t total, +/// void *baton); +/// +/// This callback will always initially be called with "completed" set to zero +/// and "total" set to the total amount specified in the contructor. This is +/// considered the progress start event. As Progress::Increment() is called, +/// the callback will be called as long as the Progress::m_completed has not +/// yet exceeded the Progress::m_total. When the callback is called with +/// Progress::m_completed == Progress::m_total, that is considered a progress +/// completed event. If Progress::m_completed is non-zero and less than +/// Progress::m_total, then this is considered a progress update event. +/// +/// This callback will be called in the destructor if Progress::m_completed is +/// not equal to Progress::m_total with the "completed" set to +/// Progress::m_total. This ensures we always send a progress completed update +/// even if the user does not. + +class Progress { +public: + /// Construct a progress object that will report information. + /// + /// The constructor will create a unique progress reporting object and + /// immediately send out a progress update by calling the installed callback + /// with completed set to zero out of the specified total. + /// + /// @param [in] total The total units of work to be done. Set this to + /// UINT64_MAX if there should be no progress displayed and just show a + /// spinning progress indicator. + /// + /// @param [in] total A printf format string that will be evaluated and will + /// becomde the title of this progress activity. + Progress(uint64_t total, const char *format, ...) + __attribute__((format(printf, 3, 4))); + + /// Destroy the progress object. + /// + /// If the progress has not yet sent a completion update, the destructor + /// will send out a notification where the completed == m_total. This ensures + /// that we always send out a progress complete notification. + ~Progress(); + + /// Increment the progress and send a notification to the intalled callback. + /// + /// If incrementing ends up exceeding m_total, m_completed will be updated + /// to match m_total and no subsequent progress notifications will be sent. + /// + /// @param [in] amount The amount to increment m_completed by. + void Increment(uint64_t amount = 1); + + /// Set the progress notification callback. + /// + /// If the callback is set to a valid non NULL value, the \a callback + /// function will be called when ever a Progress object is constructed, + /// Increment is called, or the Progress object is destroyed and has not + /// already sent a progress complete notification. + static void SetProgressCallback(lldb::ProgressCallback callback, void *baton); + +private: + void ReportProgress(); + static lldb::ProgressCallback g_callback; + static void *g_callback_baton; + static uint64_t g_progress_id; + /// The title of the progress activity. + ConstString m_message; + /// How much work ([0...m_total]) that has been completed. + std::atomic<uint64_t> m_completed; + /// Total amount of work, UINT64_MAX for non deterministic progress. + const uint64_t m_total; + /// A unique integer identifier for progress reporting. + const uint64_t m_id; + /// Set to true when progress has been reported where m_completed == m_total + /// to ensure that we don't send progress updates after progress has + /// completed. + bool m_complete = false; +}; + +} // namespace lldb_private + +#endif // LLDB_CORE_PROGRESS_H Index: lldb/include/lldb/API/SBDebugger.h =================================================================== --- lldb/include/lldb/API/SBDebugger.h +++ lldb/include/lldb/API/SBDebugger.h @@ -62,6 +62,8 @@ static void MemoryPressureDetected(); + static void SetProgressCallback(ProgressCallback callback, void *baton); + explicit operator bool() const; bool IsValid() const;
_______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits