From 6ed95945edea5f40fa5aedb3b049bb3d78dcce4e Mon Sep 17 00:00:00 2001 From: Greg Clayton Date: Sat, 22 Jan 2011 07:12:45 +0000 Subject: [PATCH] Sped up the shutdown time on MacOSX by quite a bit by making sure any threads that we spawn let us know when they are going away and that we don't timeout waiting for a message from threads that have gone away. We also now don't expect the "k" packet (kill) to send a response. This greatly speeds up debugger shutdown performance. The test suite now runs quite a bit faster. Added a fix to the variable display code that fixes the display of base classes. We were assuming the virtual or normal base class offsets were being given in bit sizes, but they were being given as character sizes, so we needed to multiply the offset by 8. This wasn't affecting the expression parser, but it was affecting the correct display of C++ class base classes and all of their children. llvm-svn: 124024 --- .../gdb-remote/GDBRemoteCommunication.cpp | 40 +++++++++++----- .../gdb-remote/GDBRemoteCommunication.h | 9 +++- .../Process/gdb-remote/ProcessGDBRemote.cpp | 47 ++++++------------- lldb/source/Symbol/ClangASTContext.cpp | 4 +- lldb/source/Symbol/ClangASTType.cpp | 4 +- lldb/source/Target/Process.cpp | 8 +--- lldb/tools/debugserver/source/RNBRemote.cpp | 9 ++-- 7 files changed, 60 insertions(+), 61 deletions(-) diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp index 92e33b257715..38b823f77aa9 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.cpp @@ -87,14 +87,23 @@ GDBRemoteCommunication::CalculcateChecksum (const char *payload, size_t payload_ } size_t -GDBRemoteCommunication::SendAck (char ack_char) +GDBRemoteCommunication::SendAck () { - Mutex::Locker locker(m_sequence_mutex); - ProcessGDBRemoteLog::LogIf (GDBR_LOG_PACKETS, "send packet: %c", ack_char); + ProcessGDBRemoteLog::LogIf (GDBR_LOG_PACKETS, "send packet: +"); ConnectionStatus status = eConnectionStatusSuccess; + char ack_char = '+'; return Write (&ack_char, 1, status, NULL) == 1; } +size_t +GDBRemoteCommunication::SendNack () +{ + ProcessGDBRemoteLog::LogIf (GDBR_LOG_PACKETS, "send packet: -"); + ConnectionStatus status = eConnectionStatusSuccess; + char nack_char = '-'; + return Write (&nack_char, 1, status, NULL) == 1; +} + size_t GDBRemoteCommunication::SendPacketAndWaitForResponse ( @@ -141,7 +150,8 @@ GDBRemoteCommunication::SendPacketAndWaitForResponse m_async_packet_predicate.SetValue (true, eBroadcastNever); bool timed_out = false; - if (SendInterrupt(locker, 1, &timed_out)) + bool sent_interrupt = false; + if (SendInterrupt(locker, 1, sent_interrupt, timed_out)) { if (m_async_packet_predicate.WaitForValueEqualTo (false, &timeout_time, &timed_out)) { @@ -443,8 +453,9 @@ GDBRemoteCommunication::SendAsyncSignal (int signo) { m_async_signal = signo; bool timed_out = false; + bool sent_interrupt = false; Mutex::Locker locker; - if (SendInterrupt (locker, 1, &timed_out)) + if (SendInterrupt (locker, 1, sent_interrupt, timed_out)) return true; m_async_signal = -1; return false; @@ -461,10 +472,16 @@ GDBRemoteCommunication::SendAsyncSignal (int signo) // (gdb remote protocol requires this), and do what we need to do, then resume. bool -GDBRemoteCommunication::SendInterrupt (Mutex::Locker& locker, uint32_t seconds_to_wait_for_stop, bool *timed_out) +GDBRemoteCommunication::SendInterrupt +( + Mutex::Locker& locker, + uint32_t seconds_to_wait_for_stop, + bool &sent_interrupt, + bool &timed_out +) { - if (timed_out) - *timed_out = false; + sent_interrupt = false; + timed_out = false; if (IsConnected() && IsRunning()) { @@ -484,8 +501,9 @@ GDBRemoteCommunication::SendInterrupt (Mutex::Locker& locker, uint32_t seconds_t ProcessGDBRemoteLog::LogIf (GDBR_LOG_PACKETS, "send packet: \\x03"); if (Write (&ctrl_c, 1, status, NULL) > 0) { + sent_interrupt = true; if (seconds_to_wait_for_stop) - m_private_is_running.WaitForValueEqualTo (false, &timeout, timed_out); + m_private_is_running.WaitForValueEqualTo (false, &timeout, &timed_out); return true; } } @@ -553,9 +571,9 @@ GDBRemoteCommunication::WaitForPacketNoLock (StringExtractorGDBRemote &response, checksum_error = packet_checksum != actual_checksum; // Send the ack or nack if needed if (checksum_error || !success) - SendAck('-'); + SendNack(); else - SendAck('+'); + SendAck(); } } else diff --git a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h index ab9384f291e0..bcbaeb5ebd91 100644 --- a/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h +++ b/lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunication.h @@ -87,7 +87,11 @@ public: GetAck (uint32_t timeout_seconds); size_t - SendAck (char ack_char); + SendAck (); + + size_t + SendNack (); + char CalculcateChecksum (const char *payload, @@ -117,7 +121,8 @@ public: bool SendInterrupt (lldb_private::Mutex::Locker &locker, uint32_t seconds_to_wait_for_stop, - bool *timed_out = NULL); + bool &sent_interrupt, + bool &timed_out); bool GetSequenceMutex(lldb_private::Mutex::Locker& locker); diff --git a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp index f0087ec42007..5abe070f9622 100644 --- a/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp +++ b/lldb/source/Plugins/Process/gdb-remote/ProcessGDBRemote.cpp @@ -541,7 +541,7 @@ ProcessGDBRemote::ConnectToDebugserver (const char *host_port) if (m_gdb_comm.StartReadThread(&error)) { // Send an initial ack - m_gdb_comm.SendAck('+'); + m_gdb_comm.SendAck(); if (m_debugserver_pid != LLDB_INVALID_PROCESS_ID) m_debugserver_thread = Host::StartMonitoringChildProcess (MonitorDebugserverProcess, @@ -1141,24 +1141,16 @@ Error ProcessGDBRemote::DoHalt (bool &caused_stop) { Error error; - - if (m_gdb_comm.IsRunning()) - { - caused_stop = true; - bool timed_out = false; - Mutex::Locker locker; - if (!m_gdb_comm.SendInterrupt (locker, 2, &timed_out)) - { - if (timed_out) - error.SetErrorString("timed out sending interrupt packet"); - else - error.SetErrorString("unknown error sending interrupt packet"); - } - } - else + bool timed_out = false; + Mutex::Locker locker; + + if (!m_gdb_comm.SendInterrupt (locker, 2, caused_stop, timed_out)) { - caused_stop = false; + if (timed_out) + error.SetErrorString("timed out sending interrupt packet"); + else + error.SetErrorString("unknown error sending interrupt packet"); } return error; @@ -1172,11 +1164,12 @@ ProcessGDBRemote::WillDetach () if (m_gdb_comm.IsRunning()) { bool timed_out = false; + bool sent_interrupt = false; Mutex::Locker locker; PausePrivateStateThread(); m_thread_list.DiscardThreadPlans(); m_debugserver_pid = LLDB_INVALID_PROCESS_ID; - if (!m_gdb_comm.SendInterrupt (locker, 2, &timed_out)) + if (!m_gdb_comm.SendInterrupt (locker, 2, sent_interrupt, timed_out)) { if (timed_out) error.SetErrorString("timed out sending interrupt packet"); @@ -1238,22 +1231,10 @@ ProcessGDBRemote::DoDestroy () log->Printf ("ProcessGDBRemote::DoDestroy()"); // Interrupt if our inferior is running... - Mutex::Locker locker; - m_gdb_comm.SendInterrupt (locker, 1); - DisableAllBreakpointSites (); - SetExitStatus(-1, "process killed"); - - StringExtractorGDBRemote response; - if (m_gdb_comm.SendPacketAndWaitForResponse("k", response, 1, false)) + if (m_gdb_comm.IsConnected()) { - log = ProcessGDBRemoteLog::GetLogIfAllCategoriesSet(GDBR_LOG_PROCESS); - if (log) - { - if (response.IsOKPacket()) - log->Printf ("ProcessGDBRemote::DoDestroy() kill was successful"); - else - log->Printf ("ProcessGDBRemote::DoDestroy() kill failed: %s", response.GetStringRef().c_str()); - } + // Don't get a response when killing our + m_gdb_comm.SendPacket ("k", 1); } StopAsyncThread (); diff --git a/lldb/source/Symbol/ClangASTContext.cpp b/lldb/source/Symbol/ClangASTContext.cpp index 9988d0860c93..7e68a637f29a 100644 --- a/lldb/source/Symbol/ClangASTContext.cpp +++ b/lldb/source/Symbol/ClangASTContext.cpp @@ -2475,9 +2475,9 @@ ClangASTContext::GetChildClangTypeAtIndex if (base_class->isVirtual()) - bit_offset = record_layout.getVBaseClassOffset(base_class_decl).getQuantity(); + bit_offset = record_layout.getVBaseClassOffset(base_class_decl).getQuantity() * 8; else - bit_offset = record_layout.getBaseClassOffset(base_class_decl).getQuantity(); + bit_offset = record_layout.getBaseClassOffset(base_class_decl).getQuantity() * 8; // Base classes should be a multiple of 8 bits in size assert (bit_offset % 8 == 0); diff --git a/lldb/source/Symbol/ClangASTType.cpp b/lldb/source/Symbol/ClangASTType.cpp index 6f6afdebf7d2..de559fdc1d0f 100644 --- a/lldb/source/Symbol/ClangASTType.cpp +++ b/lldb/source/Symbol/ClangASTType.cpp @@ -392,9 +392,9 @@ ClangASTType::DumpValue continue; if (base_class->isVirtual()) - field_bit_offset = record_layout.getVBaseClassOffset(base_class_decl).getQuantity(); + field_bit_offset = record_layout.getVBaseClassOffset(base_class_decl).getQuantity() * 8; else - field_bit_offset = record_layout.getBaseClassOffset(base_class_decl).getQuantity(); + field_bit_offset = record_layout.getBaseClassOffset(base_class_decl).getQuantity() * 8; field_byte_offset = field_bit_offset / 8; assert (field_bit_offset % 8 == 0); if (child_idx == 0) diff --git a/lldb/source/Target/Process.cpp b/lldb/source/Target/Process.cpp index 2903daddde9d..dab34765ef53 100644 --- a/lldb/source/Target/Process.cpp +++ b/lldb/source/Target/Process.cpp @@ -407,7 +407,6 @@ Process::WaitForStateChangedEvents (const TimeValue *timeout, EventSP &event_sp) event_sp)) state = Process::ProcessEventData::GetStateFromEvent(event_sp.get()); - log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS); if (log) log->Printf ("Process::%s (timeout = %p, event_sp) => %s", __FUNCTION__, @@ -427,7 +426,6 @@ Process::PeekAtStateChangedEvents () Event *event_ptr; event_ptr = m_listener.PeekAtNextEventForBroadcasterWithType (this, eBroadcastBitStateChanged); - log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS); if (log) { if (event_ptr) @@ -463,7 +461,6 @@ Process::WaitForStateChangedEventsPrivate (const TimeValue *timeout, EventSP &ev // This is a bit of a hack, but when we wait here we could very well return // to the command-line, and that could disable the log, which would render the // log we got above invalid. - log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS); if (log) log->Printf ("Process::%s (timeout = %p, event_sp) => %s", __FUNCTION__, timeout, StateAsCString(state)); return state; @@ -2140,7 +2137,6 @@ Process::RunPrivateStateThread () break; } - log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS); if (log) log->Printf ("Process::%s (arg = %p, pid = %i) got a control event: %d", __FUNCTION__, this, GetID(), event_sp->GetType()); @@ -2160,7 +2156,6 @@ Process::RunPrivateStateThread () internal_state == eStateExited || internal_state == eStateDetached ) { - log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS); if (log) log->Printf ("Process::%s (arg = %p, pid = %i) about to exit with internal state %s...", __FUNCTION__, this, GetID(), StateAsCString(internal_state)); @@ -2169,10 +2164,11 @@ Process::RunPrivateStateThread () } // Verify log is still enabled before attempting to write to it... - log = lldb_private::GetLogIfAllCategoriesSet (LIBLLDB_LOG_PROCESS); if (log) log->Printf ("Process::%s (arg = %p, pid = %i) thread exiting...", __FUNCTION__, this, GetID()); + m_private_state_control_wait.SetValue (true, eBroadcastAlways); + m_private_state_thread = LLDB_INVALID_HOST_THREAD; return NULL; } diff --git a/lldb/tools/debugserver/source/RNBRemote.cpp b/lldb/tools/debugserver/source/RNBRemote.cpp index bb1951955d34..235f9a7e047c 100644 --- a/lldb/tools/debugserver/source/RNBRemote.cpp +++ b/lldb/tools/debugserver/source/RNBRemote.cpp @@ -3123,11 +3123,10 @@ RNBRemote::HandlePacket_D (const char *p) rnb_err_t RNBRemote::HandlePacket_k (const char *p) { - if (!m_ctx.HasValidProcessID()) - return SendPacket ("E26"); - if (!DNBProcessKill (m_ctx.ProcessID())) - return SendPacket ("E27"); - return SendPacket ("OK"); + // No response to should be sent to the kill packet + if (m_ctx.HasValidProcessID()) + DNBProcessKill (m_ctx.ProcessID()); + return rnb_success; } rnb_err_t