From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103542 invoked by alias); 13 Dec 2019 15:17:12 -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 103526 invoked by uid 89); 13 Dec 2019 15:17:12 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: =?ISO-8859-1?Q?No, score=-29.9 required=5.0 tests=AWL,BAYES_00,ENV_AND_HDR_SPF_MATCH,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,HTML_MESSAGE,RCVD_IN_DNSWL_NONE,SPF_PASS,USER_IN_DEF_SPF_WL autolearn=ham version=3.3.1 spammy=HX-Received:24a6, structure.=c2?= X-HELO: mail-ot1-f67.google.com Received: from mail-ot1-f67.google.com (HELO mail-ot1-f67.google.com) (209.85.210.67) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 13 Dec 2019 15:17:08 +0000 Received: by mail-ot1-f67.google.com with SMTP id a15so6865248otf.1 for ; Fri, 13 Dec 2019 07:17:08 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=mPaMG+z2qPgt6lvwU82EztYW2ysEjTYVmKQgn0IkAFI=; b=p0uSDMbn+VGuq/FjY4f7ZTndk6vwcfprm2DbUccNGrxSyxYN4OSv5yS6c/SfV/dWl5 qdUqtNaFly6lLHsB/kZHvoKK5RgMNkP3kVrAsfy5xQ5Yp2Nhe2DlECgFtRQ0k1xgXZmD chxgjQF1+SKLYaTZNG3ElSssZnEE+/6ppUqnd1eZHCQwYGvFHPKd+Mgs0tIEpqQ6jp1a uq0ijaRSFfWDpfXyGnSSbv7aSFGINA892pEKunnrLH5lrblxpM/DyYzpigHDhWygBoTy iF0q9nE3mThfWgjXoLi45mtN0h//erCJrOP3ubMUG0o99zCH2pdHYD84knmss29JNYZ7 hrpA== MIME-Version: 1.0 References: <20191213060323.1799590-1-simon.marchi@polymtl.ca> <20191213060323.1799590-5-simon.marchi@polymtl.ca> In-Reply-To: <20191213060323.1799590-5-simon.marchi@polymtl.ca> From: "Christian Biesinger via gdb-patches" Reply-To: Christian Biesinger Date: Fri, 13 Dec 2019 15:17:00 -0000 Message-ID: Subject: Re: [PATCH 4/7] jit: make gdb_symtab::blocks a vector To: Simon Marchi Cc: gdb-patches Content-Type: text/plain; charset="UTF-8" X-IsSubscribed: yes X-SW-Source: 2019-12/txt/msg00597.txt.bz2 On Fri, Dec 13, 2019, 01:18 Simon Marchi wrote: > This patch changes the gdb_symtab::blocks linked list to be an > std::vector, simplifying memory management. > > Currently, the list is sorted as blocks are created. It is easier (and > probably a bit more efficient) to sort them once at the end, so this is > what I did. > > A note about the comment on the "next" field: > > /* gdb_blocks are linked into a tree structure. Next points to the > next node at the same depth as this block and parent to the > parent gdb_block. */ > > I don't think it's true that "next" points to the next node at the same > depth. It might happen to be true for some nodes, but it can't be true > in general, as this is a simple linked list containing all the created > blocks. > > gdb/ChangeLog: > > * jit.c (struct gdb_block) : Remove field. > (struct gdb_symtab) <~gdb_symtab>: Adjust to std::vector. > : Change type to std::vector. > : Remove. > (compare_block): Remove. > (jit_block_open_impl): Adjust to std::vector. Place the new > block at the end, don't mind about sorting. > (finalize_symtab): Adjust to std::vector, sort the blocks vector > before using it. > --- > gdb/jit.c | 111 +++++++++++++++--------------------------------------- > 1 file changed, 31 insertions(+), 80 deletions(-) > > diff --git a/gdb/jit.c b/gdb/jit.c > index eace83e583d3..bb855e09f59b 100644 > --- a/gdb/jit.c > +++ b/gdb/jit.c > @@ -428,10 +428,8 @@ jit_read_code_entry (struct gdbarch *gdbarch, > > struct gdb_block > { > - /* gdb_blocks are linked into a tree structure. Next points to the > - next node at the same depth as this block and parent to the > - parent gdb_block. */ > - struct gdb_block *next, *parent; > + /* The parent of this block. */ > + struct gdb_block *parent; > > /* Points to the "real" block that is being built out of this > instance. This block will be added to a blockvector, which will > @@ -456,14 +454,8 @@ struct gdb_symtab > > ~gdb_symtab () > { > - gdb_block *gdb_block_iter, *gdb_block_iter_tmp; > - > - for ((gdb_block_iter = this->blocks, > - gdb_block_iter_tmp = gdb_block_iter->next); > - gdb_block_iter; > - gdb_block_iter = gdb_block_iter_tmp) > + for (gdb_block *gdb_block_iter : this->blocks) > { > - gdb_block_iter_tmp = gdb_block_iter->next; > xfree ((void *) gdb_block_iter->name); > xfree (gdb_block_iter); > } > @@ -471,10 +463,7 @@ struct gdb_symtab > > /* The list of blocks in this symtab. These will eventually be > converted to real blocks. */ > - struct gdb_block *blocks = nullptr; > - > - /* The number of blocks inserted. */ > - int nblocks = 0; > + std::vector blocks; > > /* A mapping between line numbers to PC. */ > gdb::unique_xmalloc_ptr linetable; > @@ -537,28 +526,6 @@ jit_symtab_open_impl (struct gdb_symbol_callbacks *cb, > return symtab; > } > > -/* Returns true if the block corresponding to old should be placed > - before the block corresponding to new in the final blockvector. */ > - > -static int > -compare_block (const struct gdb_block *const old, > - const struct gdb_block *const newobj) > -{ > - if (old == NULL) > - return 1; > - if (old->begin < newobj->begin) > - return 1; > - else if (old->begin == newobj->begin) > - { > - if (old->end > newobj->end) > - return 1; > - else > - return 0; > - } > - else > - return 0; > -} > - > /* Called by readers to open a new gdb_block. This function also > inserts the new gdb_block in the correct place in the corresponding > gdb_symtab. */ > @@ -570,37 +537,15 @@ jit_block_open_impl (struct gdb_symbol_callbacks *cb, > { > struct gdb_block *block = XCNEW (struct gdb_block); > > - block->next = symtab->blocks; > block->begin = (CORE_ADDR) begin; > block->end = (CORE_ADDR) end; > block->name = name ? xstrdup (name) : NULL; > block->parent = parent; > > - /* Ensure that the blocks are inserted in the correct (reverse of > - the order expected by blockvector). */ > - if (compare_block (symtab->blocks, block)) > - { > - symtab->blocks = block; > - } > - else > - { > - struct gdb_block *i = symtab->blocks; > - > - for (;; i = i->next) > - { > - /* Guaranteed to terminate, since compare_block (NULL, _) > - returns 1. */ > - if (compare_block (i->next, block)) > - { > - block->next = i->next; > - i->next = block; > - break; > - } > - } > - } > - symtab->nblocks++; > - > - return block; > + /* Place the block at the end of the vector, it will be sorted when the > + symtab is finalized. */ > + symtab->blocks.push_back (block); > + return symtab->blocks.back (); > } > > /* Readers call this to add a line mapping (from PC to line number) to > @@ -646,14 +591,21 @@ static void > finalize_symtab (struct gdb_symtab *stab, struct objfile *objfile) > { > struct compunit_symtab *cust; > - struct gdb_block *gdb_block_iter; > - struct block *block_iter; > - int actual_nblocks, i; > size_t blockvector_size; > CORE_ADDR begin, end; > struct blockvector *bv; > > - actual_nblocks = FIRST_LOCAL_BLOCK + stab->nblocks; > + int actual_nblocks = FIRST_LOCAL_BLOCK + stab->blocks.size (); > + > + /* Sort the blocks in the order they should appear in the blockvector. > */ > + std::sort (stab->blocks.begin (), stab->blocks.end (), > + [] (const gdb_block *a, const gdb_block *b) > + { > + if (a->begin != b->begin) > + return a->begin < b->begin; > + > + return b->begin < a->begin; > This doesn't look right? Should this look at end or something? + }); > cust = allocate_compunit_symtab (objfile, stab->file_name.c_str ()); > allocate_symtab (cust, stab->file_name.c_str ()); > @@ -680,19 +632,18 @@ finalize_symtab (struct gdb_symtab *stab, struct > objfile *objfile) > blockvector_size); > COMPUNIT_BLOCKVECTOR (cust) = bv; > > - /* (begin, end) will contain the PC range this entire blockvector > - spans. */ > + /* At the end of this function, (begin, end) will contain the PC range > this > + entire blockvector spans. */ > BLOCKVECTOR_MAP (bv) = NULL; > - begin = stab->blocks->begin; > - end = stab->blocks->end; > + begin = stab->blocks.front ()->begin; > + end = stab->blocks.front ()->end; > BLOCKVECTOR_NBLOCKS (bv) = actual_nblocks; > > /* First run over all the gdb_block objects, creating a real block > object for each. Simultaneously, keep setting the real_block > fields. */ > - for (i = (actual_nblocks - 1), gdb_block_iter = stab->blocks; > - i >= FIRST_LOCAL_BLOCK; > - i--, gdb_block_iter = gdb_block_iter->next) > + int block_idx = FIRST_LOCAL_BLOCK; > + for (gdb_block *gdb_block_iter : stab->blocks) > { > struct block *new_block = allocate_block > (&objfile->objfile_obstack); > struct symbol *block_name = allocate_symbol (objfile); > @@ -719,18 +670,20 @@ finalize_symtab (struct gdb_symtab *stab, struct > objfile *objfile) > > BLOCK_FUNCTION (new_block) = block_name; > > - BLOCKVECTOR_BLOCK (bv, i) = new_block; > + BLOCKVECTOR_BLOCK (bv, block_idx) = new_block; > if (begin > BLOCK_START (new_block)) > begin = BLOCK_START (new_block); > if (end < BLOCK_END (new_block)) > end = BLOCK_END (new_block); > > gdb_block_iter->real_block = new_block; > + > + block_idx++; > } > > /* Now add the special blocks. */ > - block_iter = NULL; > - for (i = 0; i < FIRST_LOCAL_BLOCK; i++) > + struct block *block_iter = NULL; > + for (enum block_enum i : { GLOBAL_BLOCK, STATIC_BLOCK }) > { > struct block *new_block; > > @@ -753,9 +706,7 @@ finalize_symtab (struct gdb_symtab *stab, struct > objfile *objfile) > > /* Fill up the superblock fields for the real blocks, using the > real_block fields populated earlier. */ > - for (gdb_block_iter = stab->blocks; > - gdb_block_iter; > - gdb_block_iter = gdb_block_iter->next) > + for (gdb_block *gdb_block_iter : stab->blocks) > { > if (gdb_block_iter->parent != NULL) > { > -- > 2.24.1 > >