[FYI/PUSHED] Revert "Turn off threaded minsym demangling by default"

[FYI/PUSHED] Revert "Turn off threaded minsym demangling by default"

[cbiesinger]

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(-)

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;
 
-- 
2.24.1.735.g03f4e72817-goog

Re: [FYI/PUSHED] Revert "Turn off threaded minsym demangling by default"

[Tom Tromey]

>>>>> ">" == 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

Re: [FYI/PUSHED] Revert "Turn off threaded minsym demangling by default"

[Christian Biesinger via gdb-patches]

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

Re: [FYI/PUSHED] Revert "Turn off threaded minsym demangling by default"

[Pedro Alves]

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

Re: [FYI/PUSHED] Revert "Turn off threaded minsym demangling by default"

[Christian Biesinger via gdb-patches]

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