[review] Compute msymbol hash codes in parallel

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

Tom Tromey has posted comments on this change.

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


Patch Set 1: Code-Review+1

(3 comments)

Thank you.  I appreciate the work you've done speeding up this area.
Marking this +1 since it needs a few fixes and also depends on
the threading patches.

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/308/1/gdb/minsyms.c 
File gdb/minsyms.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/308/1/gdb/minsyms.c@1251 
PS1, Line 1251: 
1244 |       *copyto++ = *copyfrom++;
1245 |       mcount = copyto - msymbol;
1246 |     }
1247 |   return (mcount);
1248 | }
1249 | 
1250 | struct computed_hash_values {
1251 |   hashval_t mangled_name_hash;
1252 |   unsigned int minsym_hash;
1253 |   unsigned int minsym_demangled_hash;

It's a shame we need 3, but I guess the minsym ones are
case-insensitive.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/308/1/gdb/minsyms.c@1263 
PS1, Line 1263: 
1254 | };
1255 | 
1256 | /* Build (or rebuild) the minimal symbol hash tables.  This is necessary
1257 |    after compacting or sorting the table since the entries move around
1258 |    thus causing the internal minimal_symbol pointers to become jumbled.  */
1259 |   
1260 | static void
1261 | build_minimal_symbol_hash_tables
1262 |   (struct objfile *objfile,
1263 |    std::vector<computed_hash_values>& hash_values)

Could be const?


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/308/1/gdb/minsyms.c@1269 
PS1, Line 1269: 
1261 | build_minimal_symbol_hash_tables
     | ...
1264 | {
1265 |   int i;
1266 |   struct minimal_symbol *msym;
1267 | 
1268 |   /* Clear the hash tables.  */
1269 |   for (i = 0; i < MINIMAL_SYMBOL_HASH_SIZE; i++)
1270 |     {
1271 |       objfile->per_bfd->msymbol_hash[i] = 0;
1272 |       objfile->per_bfd->msymbol_demangled_hash[i] = 0;
1273 |     }

Not part of your patch but this is only needed when multiple
symbol readers install minsyms, which is pretty rare.  So,
maybe it could be moved elsewhere to speed up the normal case.

This also applies to the 2 lines in this function that clear out
the "hash_next" and "demangled_hash_next" fields, though I suspect
it's not worth much to remove these.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ifaa3346e9998f05743bff9e2eaad3f83b954d071
Gerrit-Change-Number: 308
Gerrit-PatchSet: 1
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 29 Oct 2019 19:44:27 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment