public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Richard Biener <richard.guenther@gmail.com>
To: Steven Bosscher <stevenb.gcc@gmail.com>
Cc: Michael Matz <matz@suse.de>,
	GCC Patches <gcc-patches@gcc.gnu.org>, Jeff Law <law@redhat.com>
Subject: Re: [patch][RFC] bitmaps as lists *or* trees
Date: Wed, 06 Mar 2013 10:18:00 -0000	[thread overview]
Message-ID: <CAFiYyc1cGPLTfkSOhmiXfRjABF0bcBKrF8e3kTaYf9dc8xH73g@mail.gmail.com> (raw)
In-Reply-To: <CABu31nNZL70Xh8DdfB_Bejc=pAmhTseW0NDENmOtV4MAOxvuUg@mail.gmail.com>

On Tue, Mar 5, 2013 at 5:16 PM, Steven Bosscher <stevenb.gcc@gmail.com> wrote:
> On Tue, Mar 5, 2013 at 3:48 PM, Michael Matz wrote:
>> Iteration isn't easy on trees without a pointer to the parent (i.e.
>> enlarging each node), you need to remember variably sized context in the
>> iterator (e.g. the current stack of nodes).
>
> I was thinking of just making the "tree" a single forward-linked list,
> that's a valid splay tree and one that is compatible with the
> iterators, at least as long as the iterated bitmap are not modified
> (but I think that isn't allowed anyway?). Obviously the tree is not
> very well balanced after that, but splay trees have a way of
> re-balancing themselves quickly. The other possibility is to create
> vecs of bitmap_elements up front, but such iterators (fat iterators,
> as Richi just now called them in another mail :-) have the downside
> that you must always visit all bitmap elements, even if you want to be
> able to break the iteration before reaching the end.
>
>> I do like the idea of reusing the same internal data structure to
>> implement the tree.  And I'm wondering about performance impact, I
>> wouldn't be surprised either way (i.e. that it brings about a large
>> improvement, or none at all), most bitmap membership tests in GCC are
>> surprisingly clustered so that the bitmaps cache of last accessed
>> element can work its magic (not all of them, as the testcase shows of
>> course :) ).
>
> I've retained the cached last accessed element. In fact it's now
> cached twice, because the root of the splay tree is always the last
> accessed element. I've considered *not* updating bitmap->current if
> bitmap_tree_find_element doesn't find the element it's looking for.
> That way, the last accessed element would be the element that was
> *really* last accessed, i.e. with a valid membership test, instead of
> the element closest to the last tested bit. Not sure if that'd be a
> good idea.
>
> Anyway, updated patch attached.  It compiles all my cc1-i files, and
> it compiles the PR55135 test case, in 410s (I terminated cc1plus
> unpatched after >7200s, more than 2 hours...).
>
> An interesting question is, how can you identify bitmaps that could
> benefit from viewing it as a tree (at least for some time). I have no
> ideas here. Something with detailed memory stats, of course, but then
> how to derive some measure for a trade-off between list or tree view.
> Thoughts?

That's indeed a good question.  The mem-stats help to print a useful
location for where the bitmap was allocated.  But whether to switch it
or not is more fine-grained - any iteration / bitwise op should "reset"
the stats we collect.  For the rest I'd simply count # of elements
walked for the find vs. # of splays done (where of course a splay is
more expensive than a linked list element walk).  That "vs." of course
doesn't work because we don't have both views at the same time,
but counting the list walks vs. the number of searches would be a start
(doesn't help deciding whether a switch to a tree was bad and should
be undone, of course ... we'd have to compute the "distance" in
bitmap_elts between successive searches).

What about simply providing a bitmap_test_set_only () with no
way to switch back and let the tree balance itself?  Does the
splay () overhead matter?

Richard.

> Ciao!
> Steven

  parent reply	other threads:[~2013-03-06 10:18 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-03-05 12:01 Steven Bosscher
2013-03-05 12:32 ` Richard Biener
2013-03-05 14:48   ` Michael Matz
2013-03-05 16:17     ` Steven Bosscher
2013-03-05 22:56       ` Steven Bosscher
2013-03-07 20:18         ` Steven Bosscher
2018-10-17 14:04           ` Richard Biener
2013-03-06 10:18       ` Richard Biener [this message]
2013-03-06 16:02       ` Jan Hubicka
2013-03-06 16:06         ` Jan Hubicka
2013-03-06 18:07           ` Steven Bosscher
2013-03-06 17:59         ` Steven Bosscher
2013-03-06 18:03           ` Jan Hubicka
2013-03-05 14:51   ` Steven Bosscher
2013-03-05 16:03     ` Richard Biener

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAFiYyc1cGPLTfkSOhmiXfRjABF0bcBKrF8e3kTaYf9dc8xH73g@mail.gmail.com \
    --to=richard.guenther@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=law@redhat.com \
    --cc=matz@suse.de \
    --cc=stevenb.gcc@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).