Re: [cxx-conversion] New Hash Table (issue6244048)

[Lawrence Crowl <crowl at google dot com>]

On 5/28/12, Jakub Jelinek <jakub@redhat.com> wrote:
> On Fri, May 25, 2012 at 02:42:39PM -0700, Lawrence Crowl wrote:
> > On 5/24/12, Jakub Jelinek <jakub@redhat.com> wrote:
> > > On Thu, May 24, 2012 at 09:43:42AM -0700, Lawrence Crowl wrote:
> > > > Add a type-safe hash table, typed_htab.  Uses of this table
> > > > replace uses of libiberty's htab_t.  The benefits include
> > > > less boiler-plate code, full type safety, and improved
> > > > performance.
> > >
> > > You haven't looked at the most important problem of that
> > > approach - code bloat.
> >
> > Are you claiming that the size of the binary is more important
> > than run-time performance, type safety, and source code size?
>
> Runtime performance goes in hand with the size of the binary,
> at least size of frequently used code.

Well, yes and no.  We need to worry about total size, memory resident
size, and cache resident size.  The patch clearly increases total
size, but I doubt that is much of a factor because most systems
lazily load pages from the image.  I don't think we have enough
information to assess the memory resident size, and I don't think it
matters because large compilations are large because the data space
is much larger than the code space.  The patch reduces the cache
resident size because the dynamic path of instructions is shorter.

> By converting just a couple of hash tables you can't really
> measure it, you'd need to convert a significant number of them,
> then you can see what effect it has on runtime performance.

Well, I have a performance improvement with eight of them, of which
one isn't exercised in the bootstrap.

> As said earlier, GCC has lots of hash tables, and many of them
> are used in performance critical code, by increasing the I-cache
> footprint of that performance criticial code there is risk of
> reducing performance.  The common C++ programming techniques often
> lead to significant code bloat which really shouldn't be ignored.

But the patch potentially reduces the I-cache footprint.  The new
implementation eliminates pointer tests, it turns indirect
function calls into direct function calls, it enables inlining
those functions, etc.  If the compiler is suboptimal in dealing
with that, we should fix the compiler.  (I think the compiler is
behaving reasonably.)

If changing a table does not deliver performance, we can choose
not to convert that table.  We do not need to convert all of them.

-- 
Lawrence Crowl