[FYI/PUSHED] Revert "Turn off threaded minsym demangling by default"
Commit Message
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(-)
Comments
>>>>> ">" == 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
@@ -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
@@ -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. */
@@ -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;