[review] [RFC] Don't block on finishing demangling msymbols

["Tom Tromey (Code Review)" <gerrit@gnutoolchain-gerrit.osci.io>]

Tom Tromey has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/463
......................................................................


Patch Set 1:

(2 comments)

Sorry about the delay on replying to this.

| --- gdb/minsyms.c
| +++ gdb/minsyms.c
| @@ -335,18 +335,19 @@ lookup_minimal_symbol (const char *name, const char *sfile,
|  	{
|  	  if (symbol_lookup_debug)
|  	    {
|  	      fprintf_unfiltered (gdb_stdlog,
|  				  "lookup_minimal_symbol (%s, %s, %s)\n",
|  				  name, sfile != NULL ? sfile : "NULL",
|  				  objfile_debug_name (objfile));
|  	    }
|  
| +	  objfile->per_bfd->wait_for_msymbols ();

PS1, Line 344:

Ideally you'd shrink it down to just a single object I guess.

The memoized future is here
https://github.com/tromey/gdb/commit/ca3ff45f0d8e373f4da74504c2d48aa4c
92a44d5

The thread-pool change to help with this is here:
https://github.com/tromey/gdb/commit/3aa5f2cde3b4c2ae5a1af216bbbd8924c
182a077

Both are rough drafts since who knows if I'll end up using them
for psymtabs or not (recently I started leaning towards "not"
due to some psymtab weirdnesses).

|  	  /* Do two passes: the first over the ordinary hash table,
|  	     and the second over the demangled hash table.  */
|  	  lookup_minimal_symbol_mangled (name, sfile, objfile,
|  					 objfile->per_bfd->msymbol_hash,
|  					 mangled_hash, mangled_cmp, found);
|  
|  	  /* If not found, try the demangled hash table.  */
|  	  if (found.external_symbol.minsym == NULL)
|  	    {

 ...

| @@ -1410,12 +1388,11 @@ #endif
| -	       hash_values[idx].minsym_demangled_hash
| -		 = search_name_hash (MSYMBOL_LANGUAGE (msym),
| -				     MSYMBOL_SEARCH_NAME (msym));
| -	     }
| -	   {
| -	     /* To limit how long we hold the lock, we only acquire it here
| -	        and not while we demangle the names above.  */
| +      objfile_per_bfd_storage* per_bfd = m_objfile->per_bfd;
| +
| +      m_objfile->per_bfd->m_minsym_future
| +	= gdb::thread_pool::g_thread_pool->post_task ([msymbols, mcount,
| +						      per_bfd] ()

PS1, Line 1392:

> I purposely didn't use a default capture here to reduce the risk of that happening. I'm not sure I understand what you mean with the work stealing?

I meant, if you push a job into the thread pool, and then it
pushes further jobs, ideally that outer job could be some kind
of state machine so that it doesn't take up a thread pool slot
just waiting for the child jobs to finish.

> By the way, in which circumstance does an objfile have a short lifetime?

If the inferior does a dlopen followed by a dlclose, gdb will read
the objfile and then discard it.  If work is happening in the
background,
and the results aren't immediately needed, then my worry would be
the objfile being deleted while the worker is processing.

On my psymtabs-in-the-background branch, I just have the worker
keep a shared_ptr reference to the objfile to avoid this.
Maybe we could implement some kind of cancellation idea (but
not, like, async cancellation) to reduce the amount of useless
work in these cases.

| +        {
|  #if CXX_STD_THREAD
| -	     std::lock_guard<std::mutex> guard (demangled_mutex);
| +	  /* Mutex that is used when modifying or accessing the demangled
| +	     hash table.  */
| +	  std::mutex demangled_mutex;
|  #endif
| -	     for (minimal_symbol *msym = start; msym < end; ++msym)
| -	       {

-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: I9d871917459ece0b41d31670b3c56600757aea66
Gerrit-Change-Number: 463
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Christian Biesinger <cbiesinger@google.com>
Gerrit-CC: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 26 Nov 2019 21:50:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Christian Biesinger <cbiesinger@google.com>
Comment-In-Reply-To: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: comment