Message ID | 20191213224519.113253-1-cbiesinger@chromium.org |
---|---|
State | New |
Headers | show |
Series |
|
Related | show |
>>>>> ">" == cbiesinger <cbiesinger@chromium.org> writes: >> As agreed on the mailing list, now that GDB 9 has branched, this patch >> reverts the change to set worker-threads to zero. After this patch, >> multithreaded minsym demangling will be enabled again by default. Thanks. I think maybe the docs needed a tweak as well? Tom
On Fri, Dec 13, 2019 at 6:16 PM Tom Tromey <tom@tromey.com> wrote: > > >>>>> ">" == cbiesinger <cbiesinger@chromium.org> writes: > > >> As agreed on the mailing list, now that GDB 9 has branched, this patch > >> reverts the change to set worker-threads to zero. After this patch, > >> multithreaded minsym demangling will be enabled again by default. > > Thanks. I think maybe the docs needed a tweak as well? I only changed the NEWS entry that mentioned the default value. I was wondering about that -- should I change that to say the default is now back to unlimited or no? I see now that I should have uipdated gdb.texinfo in the original revert to update the default value. I'll send a patch to fix that for the branch. Do I understand it correctly that branch merges need a bug report filed? Christian
On 12/14/19 6:23 PM, Christian Biesinger via gdb-patches wrote: > On Fri, Dec 13, 2019 at 6:16 PM Tom Tromey <tom@tromey.com> wrote: >> >>>>>>> ">" == cbiesinger <cbiesinger@chromium.org> writes: >> >>>> As agreed on the mailing list, now that GDB 9 has branched, this patch >>>> reverts the change to set worker-threads to zero. After this patch, >>>> multithreaded minsym demangling will be enabled again by default. >> >> Thanks. I think maybe the docs needed a tweak as well? > > I only changed the NEWS entry that mentioned the default value. I was > wondering about that -- should I change that to say the default is now > back to unlimited or no? There should be a new entry in the "Changes since GDB 9" section, talking about multhreaded debugging now being enabled by default. The "Changes in GDB 9" entry should remain the same as it was in GDB 9, since that's how GDB 9 behaved. Thanks, Pedro Alves
On Mon, Dec 16, 2019 at 1:17 PM Pedro Alves <palves@redhat.com> wrote: > > On 12/14/19 6:23 PM, Christian Biesinger via gdb-patches wrote: > > On Fri, Dec 13, 2019 at 6:16 PM Tom Tromey <tom@tromey.com> wrote: > >> > >>>>>>> ">" == cbiesinger <cbiesinger@chromium.org> writes: > >> > >>>> As agreed on the mailing list, now that GDB 9 has branched, this patch > >>>> reverts the change to set worker-threads to zero. After this patch, > >>>> multithreaded minsym demangling will be enabled again by default. > >> > >> Thanks. I think maybe the docs needed a tweak as well? > > > > I only changed the NEWS entry that mentioned the default value. I was > > wondering about that -- should I change that to say the default is now > > back to unlimited or no? > > There should be a new entry in the "Changes since GDB 9" section, > talking about multhreaded debugging now being enabled by default. > The "Changes in GDB 9" entry should remain the same as it was in > GDB 9, since that's how GDB 9 behaved. OK, I sent https://sourceware.org/ml/gdb-patches/2019-12/msg00843.html, let me know what yall think. Christian
diff --git a/gdb/maint.c b/gdb/maint.c index f71cb80cec..51b803afab 100644 --- a/gdb/maint.c +++ b/gdb/maint.c @@ -845,12 +845,7 @@ maintenance_set_profile_cmd (const char *args, int from_tty, } #endif -static int n_worker_threads = 0; - -bool worker_threads_disabled () -{ - return n_worker_threads == 0; -} +static int n_worker_threads = -1; /* Update the thread pool for the desired number of threads. */ static void diff --git a/gdb/maint.h b/gdb/maint.h index cbaf9deaa8..827964d825 100644 --- a/gdb/maint.h +++ b/gdb/maint.h @@ -26,8 +26,6 @@ extern void set_per_command_time (int); extern void set_per_command_space (int); -extern bool worker_threads_disabled (); - /* Records a run time and space usage to be used as a base for reporting elapsed time or change in space. */ diff --git a/gdb/minsyms.c b/gdb/minsyms.c index 4f7260b380..40bedbd3e7 100644 --- a/gdb/minsyms.c +++ b/gdb/minsyms.c @@ -54,7 +54,6 @@ #include <algorithm> #include "safe-ctype.h" #include "gdbsupport/parallel-for.h" -#include "maint.h" #if CXX_STD_THREAD #include <mutex> @@ -1138,15 +1137,6 @@ minimal_symbol_reader::record_full (gdb::string_view name, else msymbol->name = name.data (); - if (worker_threads_disabled ()) - { - /* To keep our behavior as close as possible to the previous non-threaded - behavior for GDB 9.1, we call symbol_set_names here when threads - are disabled. */ - symbol_set_names (msymbol, msymbol->name, false, m_objfile->per_bfd); - msymbol->name_set = 1; - } - SET_MSYMBOL_VALUE_ADDRESS (msymbol, address); MSYMBOL_SECTION (msymbol) = section;
From: Christian Biesinger <cbiesinger@google.com> This reverts commit 62e77f56f0ce8b10122881d8f0acd70e113fde93. (except for ChangeLog and a bugfix in minimal_symbol_reader::install) As agreed on the mailing list, now that GDB 9 has branched, this patch reverts the change to set worker-threads to zero. After this patch, multithreaded minsym demangling will be enabled again by default. gdb/ChangeLog: 2019-12-13 Christian Biesinger <cbiesinger@google.com> * maint.c (n_worker_threads): Default to -1. (worker_threads_disabled): Remove function. * maint.h (worker_threads_disabled): Remove function. * minsyms.c (minimal_symbol_reader::record_full): Don't call symbol_set_names here if worker_threads_disabled () is true. Change-Id: I5ff3e318d96f60968c8b8bedb84546ad2314d94b --- gdb/maint.c | 7 +------ gdb/maint.h | 2 -- gdb/minsyms.c | 10 ---------- 3 files changed, 1 insertion(+), 18 deletions(-) -- 2.24.1.735.g03f4e72817-goog