From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 43987 invoked by alias); 29 Oct 2019 21:56:07 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 43926 invoked by uid 89); 29 Oct 2019 21:56:06 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-0.4 required=5.0 tests=BAYES_00,KAM_STOCKGEN autolearn=no version=3.3.1 spammy= X-HELO: mx1.osci.io Received: from polly.osci.io (HELO mx1.osci.io) (8.43.85.229) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Tue, 29 Oct 2019 21:56:05 +0000 Received: by mx1.osci.io (Postfix, from userid 994) id EA7F220D63; Tue, 29 Oct 2019 17:56:03 -0400 (EDT) Received: from gnutoolchain-gerrit.osci.io (gnutoolchain-gerrit.osci.io [IPv6:2620:52:3:1:5054:ff:fe06:16ca]) by mx1.osci.io (Postfix) with ESMTP id 4D22920D4C; Tue, 29 Oct 2019 17:56:01 -0400 (EDT) Received: from localhost (localhost [127.0.0.1]) by gnutoolchain-gerrit.osci.io (Postfix) with ESMTP id 31C5A2A7D9; Tue, 29 Oct 2019 17:56:01 -0400 (EDT) X-Gerrit-PatchSet: 2 Date: Tue, 29 Oct 2019 21:56:00 -0000 From: "Christian Biesinger (Code Review)" To: Christian Biesinger , gdb-patches@sourceware.org Cc: Tom Tromey Auto-Submitted: auto-generated X-Gerrit-MessageType: comment Subject: [review v2] Precompute hash value for symbol_set_names X-Gerrit-Change-Id: I044449e7eb60cffc1c43efd3412f2b485bd9faac X-Gerrit-Change-Number: 307 X-Gerrit-ChangeURL: X-Gerrit-Commit: e75f4e0d6476c51c7fe444343cf064f965c24039 In-Reply-To: References: X-Gerrit-Comment-Date: Tue, 29 Oct 2019 17:56:00 -0400 Reply-To: gnutoolchain-gerrit@osci.io MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Content-Disposition: inline User-Agent: Gerrit/3.0.3-74-g460fb0f7e9 Content-Type: text/plain; charset=UTF-8 Message-Id: <20191029215601.31C5A2A7D9@gnutoolchain-gerrit.osci.io> X-SW-Source: 2019-10/txt/msg01068.txt.bz2 Christian Biesinger has posted comments on this change. Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/307 ...................................................................... Uploaded patch set 2. (4 comments) https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/307/1/gdb/minsyms.c File gdb/minsyms.c: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/307/1/gdb/minsyms.c@1252 PS1, Line 1252: 1243 | else 1244 | *copyto++ = *copyfrom++; 1245 | } 1246 | *copyto++ = *copyfrom++; 1247 | mcount = copyto - msymbol; 1248 | } 1249 | return (mcount); 1250 | } 1251 | 1252 | struct computed_hash_values { > { on next line. Done https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/307/1/gdb/minsyms.c@1392 PS1, Line 1392: 1370 | #endif | ... 1383 | /* This will be freed later, by symbol_set_names. */ 1384 | char *demangled_name 1385 | = symbol_find_demangled_name (msym, msym->name); 1386 | symbol_set_demangled_name 1387 | (msym, demangled_name, 1388 | &m_objfile->per_bfd->storage_obstack); 1389 | msym->name_set = 1; 1390 | 1391 | size_t idx = msym - msymbols; 1392 | hash_values[idx].mangled_name_hash = htab_hash_string (msym->name); > over-long line? Also, fast_hash Great catch, thanks! Since I now need strlen (msym->name) in two places, I decided to store that in hash_values[idx] as well. Let me know if you dislike that. https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/307/1/gdb/symtab.h File gdb/symtab.h: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/307/1/gdb/symtab.h@518 PS1, Line 518: 509 | it. Used for constructs which do not have a mangled name, 510 | e.g. struct tags. Unlike SYMBOL_SET_NAMES, linkage_name must 511 | be terminated and either already on the objfile's obstack or 512 | permanently allocated. */ 513 | #define SYMBOL_SET_LINKAGE_NAME(symbol,linkage_name) \ 514 | (symbol)->ginfo.name = (linkage_name) 515 | 516 | /* Set the linkage and natural names of a symbol, by demangling 517 | the linkage name. Optionally, HASH can be set to the value 518 | of htab_hash_string (linkage_name) (if nullterminated), to > This has to refer to `fast_hash` now. Done https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/307/1/gdb/symtab.h@526 PS1, Line 526: 517 | the linkage name. Optionally, HASH can be set to the value 518 | of htab_hash_string (linkage_name) (if nullterminated), to 519 | speed up this function. */ 520 | #define SYMBOL_SET_NAMES(symbol,linkage_name,len,copy_name,objfile) \ 521 | symbol_set_names (&(symbol)->ginfo, linkage_name, len, copy_name, \ 522 | (objfile)->per_bfd) 523 | extern void symbol_set_names (struct general_symbol_info *symbol, 524 | const char *linkage_name, int len, bool copy_name, 525 | struct objfile_per_bfd_storage *per_bfd, 526 | hashval_t hash = 0); > I suppose an optional would avoid the problem […] Done -- Gerrit-Project: binutils-gdb Gerrit-Branch: master Gerrit-Change-Id: I044449e7eb60cffc1c43efd3412f2b485bd9faac Gerrit-Change-Number: 307 Gerrit-PatchSet: 2 Gerrit-Owner: Christian Biesinger Gerrit-Reviewer: Tom Tromey Gerrit-Comment-Date: Tue, 29 Oct 2019 21:56:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: Tom Tromey Gerrit-MessageType: comment