This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[review v2] Compute msymbol hash codes in parallel


Christian Biesinger has posted comments on this change.

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


Uploaded patch set 2: Patch Set 1 was rebased.

(3 comments)

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 […]

Yeah, that made me sad too :(


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?

Done


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 […]

I'll make a separate patch for that, though I don't recall that showing up in a profile.



-- 
Gerrit-Project: binutils-gdb
Gerrit-Branch: master
Gerrit-Change-Id: Ifaa3346e9998f05743bff9e2eaad3f83b954d071
Gerrit-Change-Number: 308
Gerrit-PatchSet: 2
Gerrit-Owner: Christian Biesinger <cbiesinger@google.com>
Gerrit-Reviewer: Tom Tromey <tromey@sourceware.org>
Gerrit-Comment-Date: Tue, 29 Oct 2019 21:56:00 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: Tom Tromey <tromey@sourceware.org>
Gerrit-MessageType: comment


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]