From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-f54.google.com (mail-wr1-f54.google.com [209.85.221.54]) by sourceware.org (Postfix) with ESMTPS id B72183858C83 for ; Tue, 26 Apr 2022 18:40:25 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org B72183858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=palves.net Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=gmail.com Received: by mail-wr1-f54.google.com with SMTP id w4so26632191wrg.12 for ; Tue, 26 Apr 2022 11:40:25 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:message-id:date:mime-version:user-agent:subject :content-language:to:cc:references:from:in-reply-to :content-transfer-encoding; bh=OK/BpyToLkJwdJTb2FDG/E2HQjXIFdtgj382gHj68tY=; b=IR/eW/MYE0MHBU1Gl/TKbehOrhOTWU6GfHPvjoaBXqXyaLNO2T9pRcGzuCm9y7JnR8 eUlFy2JsvBrnYpGq0vFw73Izm86BOjoSLQ6G6ocJOsF4GsY92Az7JVVy2khfZWAFD3KU h+FCLL405QWN6dBWBU5YBuC0oZO+65Da8sa6e2Z5QE3Wwx6cPHTEOsK/1c/lOlIluIR7 ISmZsFNWc0oD4FF30vbqC/maOpAgHRvPszsaGvwYPSDgd4hLPV1Zyv+LQhllxtBtO74k gTtUuaZjbRG1h3imgaIfRJj9Fq6N4VucWgHXfsx7D47WWjzb5oJ8gEjxbCK/wqQIlEK1 D0QA== X-Gm-Message-State: AOAM532IFnUaoF4QfH+L1TQWuzRTJ5/fazU3gaKpchPsIgOygxP4FS4m DeWoSonqAmi4huHPZfuznNUSzfPsPNU= X-Google-Smtp-Source: ABdhPJwXaBD4gqZCKt2b/37Te/WSIZHyV1AXRZI9pvQAcJck83RS5sDqJnR56GYWwJAT9q5WJX/YpA== X-Received: by 2002:adf:eb09:0:b0:207:bb77:9abb with SMTP id s9-20020adfeb09000000b00207bb779abbmr19459100wrn.375.1650998424358; Tue, 26 Apr 2022 11:40:24 -0700 (PDT) Received: from ?IPV6:2001:8a0:f924:2600:209d:85e2:409e:8726? ([2001:8a0:f924:2600:209d:85e2:409e:8726]) by smtp.gmail.com with ESMTPSA id v6-20020a056000144600b0020a9a1627e2sm12421890wrx.15.2022.04.26.11.40.22 (version=TLS1_3 cipher=TLS_AES_128_GCM_SHA256 bits=128/128); Tue, 26 Apr 2022 11:40:23 -0700 (PDT) Message-ID: <5ec4d8e1-9c16-c73a-ec8b-7802b498ba9b@palves.net> Date: Tue, 26 Apr 2022 19:40:22 +0100 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:91.0) Gecko/20100101 Thunderbird/91.8.0 Subject: Re: [PATCH] Fix method naming bug in new DWARF indexer Content-Language: en-US To: Tom Tromey Cc: gdb-patches@sourceware.org References: <20220421163831.2582161-1-tromey@adacore.com> <02551264-3beb-0348-07da-e61dbf9681c8@palves.net> <87mtg9ow5w.fsf@tromey.com> From: Pedro Alves In-Reply-To: <87mtg9ow5w.fsf@tromey.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.7 required=5.0 tests=BAYES_00, FREEMAIL_FORGED_FROMDOMAIN, FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_STATUS, NICE_REPLY_A, RCVD_IN_DNSWL_NONE, RCVD_IN_MSPIKE_H2, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.4 X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 26 Apr 2022 18:40:27 -0000 On 2022-04-25 19:27, Tom Tromey wrote: > Pedro> The "much slower" aspect remains, this patch does not change it. > Pedro> The size of the generated index roughly the same as from before > Pedro> this patch, and it is still much larger than from before the new > Pedro> DWARF indexer landed. I think that will require a separate fix. > Pedro> I sent you more info > Pedro> about it off list. > > I have a patch to fix the size problem. It works by only emitting a > given type or variable once. This is what the gdb used to do via the > psymbol bcache. It could sometimes mean that "info types" would miss > a duplicate -- but the old code did this as well. > > I'll send this soon. > Thanks, I saw it on the list now, and I gave it a try. > However, this patch doesn't fix the performance problem. I am not sure > it is really fixable, given that the code has to compute the full name > of every entry in the index. > Hmm, it actually fixes most of the performance for me. With that new patch (and I guess the linkage names patch made a difference too), I see roughly the same startup time in new gdb vs old gdb when using the same index, and either an index generated by old gdb, or by new gdb. Generating the index is a bit slower than before, 7.1s vs 6.6s, a rough 8% slowdown, but then again, the generated index is still ~13% bigger than the one generated by old gdb (17MB vs 15MB), which I assume explains the remaining index generation slowdown. The patch you point at addresses variables/enums, and I wonder whether the remainder of size increase (and thus the remainder of the slowdown) is explained by functions. In the old indexer, with an index of a gdb from before the indexer rewrite, we had: [369902] intrusive_list >::begin: 9 [global, function] 255 [global, function] while in the new indexer, for the same function, we have: [369902] intrusive_list >::begin: 9 [global, function] 79 [global, function] 165 [global, function] 167 [global, function] 252 [global, function] 253 [global, function] 254 [global, function] 255 [global, function] 263 [global, function] 266 [global, function] 310 [global, function] 311 [global, function] 373 [global, function] 376 [global, function] 396 [global, function] 436 [global, function] 503 [global, function] 506 [global, function] 507 [global, function] 513 [global, function] Same with "make_scoped_restore", for instance -- one entry in old index, and 21 entries in new index. Now, if I load that old gdb under new gdb, and set a breakpoint at one of those functions, I get as many locations as entries in the old indexes. For example: $ g="./gdb -data-directory=data-directory" $ $g -nx /home/pedro/slowdown/gdb.before-indexer [...] (gdb) b intrusive_list >::begin Breakpoint 2 at 0x555555c9aefe: intrusive_list >::begin. (2 locations) ^^^^^^^^^^^ Note the 2 locations found. "2" matches the number of entries in the old index for that function. (gdb) info breakpoints Num Type Disp Enb Address What 2 breakpoint keep y 2.1 y 0x0000555555c9aefe in intrusive_list >::begin() const at ../../src/gdb/../gdbsupport/intrusive_list.h:521 2.2 y 0x00005555560ab946 in intrusive_list >::begin() at ../../src/gdb/../gdbsupport/intrusive_list.h:516 Same for "make_scoped_restore" -- setting a breakpoint there only finds one location, just like there's only one entry for that function in the old index: (gdb) b make_scoped_restore Breakpoint 1 at 0x83facc: file ../../src/gdb/../gdbsupport/scoped_restore.h, line 115. (gdb) info breakpoints Num Type Disp Enb Address What 1 breakpoint keep y 0x0000555555d93acc in make_scoped_restore(int*, int) at ../../src/gdb/../gdbsupport/scoped_restore.h:115 If we used the same htab dedup logic as in your variables/enums patch, for functions, then we'd end up with only a single entry in the index for the intrusive_list example, which seems incorrect. What was the logic that the old writer used to come up with the seemingly right number of function entries? > If we check in the background-writing code, this won't be noticeable for > the index cache case. However, it will still be noticeable for > "gdb-add-index". For the time being, I'll continue using indexes, because as soon as you need to run to a breakpoint from the command line, which I do all the time, then having an index still beats the new scanner. E.g., for me: g="./gdb -data-directory=data-directory" $ time $g -q --batch -iex "set index-cache enabled off" $g -ex "start" Setting up the environment for debugging gdb. Breakpoint 1 at 0xa364b4: file /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc, line 51. Breakpoint 2 at 0x260fa3: file /home/pedro/gdb/binutils-gdb/src/gdb/cli/cli-cmds.c, line 217. Temporary breakpoint 3 at 0xeb06c: file /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c, line 25. [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Temporary breakpoint 3, main (argc=1, argv=0x7fffffffdc48) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25 25 { real 0m3.920s user 0m6.924s sys 0m0.245s vs (w/ hot index cache): $ time $g -q --batch -iex "set index-cache enabled on" $g -ex "start" Setting up the environment for debugging gdb. Breakpoint 1 at 0xa364b4: file /home/pedro/gdb/binutils-gdb/src/gdbsupport/errors.cc, line 51. Breakpoint 2 at 0x260fa3: file /home/pedro/gdb/binutils-gdb/src/gdb/cli/cli-cmds.c, line 217. Temporary breakpoint 3 at 0xeb06c: file /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c, line 25. [Thread debugging using libthread_db enabled] Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1". Temporary breakpoint 3, main (argc=1, argv=0x7fffffffdc48) at /home/pedro/gdb/binutils-gdb/src/gdb/gdb.c:25 25 { real 0m1.431s user 0m2.395s sys 0m0.091s