From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <gcc-patches-return-281499-listarch-gcc-patches=gcc.gnu.org@gcc.gnu.org>
Received: (qmail 25614 invoked by alias); 21 Dec 2010 01:31:12 -0000
Received: (qmail 25604 invoked by uid 22791); 21 Dec 2010 01:31:11 -0000
X-SWARE-Spam-Status: No, hits=-2.2 required=5.0	tests=AWL,BAYES_00,DKIM_SIGNED,DKIM_VALID,FREEMAIL_FROM,RCVD_IN_DNSWL_LOW
X-Spam-Check-By: sourceware.org
Received: from mail-qy0-f182.google.com (HELO mail-qy0-f182.google.com) (209.85.216.182)    by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 21 Dec 2010 01:31:05 +0000
Received: by qyk36 with SMTP id 36so3626234qyk.20        for <gcc-patches@gcc.gnu.org>; Mon, 20 Dec 2010 17:31:03 -0800 (PST)
MIME-Version: 1.0
Received: by 10.229.181.9 with SMTP id bw9mr4341400qcb.143.1292895063816; Mon, 20 Dec 2010 17:31:03 -0800 (PST)
Received: by 10.229.75.70 with HTTP; Mon, 20 Dec 2010 17:31:03 -0800 (PST)
In-Reply-To: <1291979498-1604-7-git-send-email-dodji@redhat.com>
References: <1291979498-1604-1-git-send-email-dodji@redhat.com>	<1291979498-1604-7-git-send-email-dodji@redhat.com>
Date: Tue, 21 Dec 2010 07:30:00 -0000
Message-ID: <AANLkTi=jaLHT_Kc3eOVA6mfZvBNUSZ-jgatvtXjJFhuG@mail.gmail.com>
Subject: Re: [PATCH 5/6] Add line map statistics to -fmem-report output
From: Gabriel Dos Reis <gdr@integrable-solutions.net>
To: Dodji Seketeli <dodji@redhat.com>
Cc: gcc-patches@gcc.gnu.org, tromey@redhat.com, joseph@codesourcery.com, 	lopezibanez@gmail.com
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable
X-IsSubscribed: yes
Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm
Precedence: bulk
List-Id: <gcc-patches.gcc.gnu.org>
List-Archive: <http://gcc.gnu.org/ml/gcc-patches/>
List-Post: <mailto:gcc-patches@gcc.gnu.org>
List-Help: <mailto:gcc-patches-help@gcc.gnu.org>
Sender: gcc-patches-owner@gcc.gnu.org
X-SW-Source: 2010-12/txt/msg01597.txt.bz2

On Fri, Dec 10, 2010 at 5:11 AM, Dodji Seketeli <dodji@redhat.com> wrote:
> This patch adds statistics about the memory consumption of line maps
> to the output of -fmem-report. It has been useful in trying to reduce
> the memory consumption of the macro maps support.
>
> Tested on x86_64-unknown-linux-gnu against trunk.
>
> gcc/
> =A0 =A0 =A0 =A0* input.c (SCALE, STAT_LABEL, FORMAT_AMOUNT): New macros.
> =A0 =A0 =A0 =A0(dump_line_table_statistics): Define new function.
> =A0 =A0 =A0 =A0* input.h (dump_line_table_statistics): Declare new functi=
on.
> =A0 =A0 =A0 =A0* toplev.c (dump_memory_report): Call dump_line_table_stat=
istics.

Can we give these `1024' some meaningfull symbolic names?
SCALE is a bit a vague -- one has to look into its body to
understand what it is doing, which defeats the purpose of abstraction.
Also, those macro should be documented.

Finally, this:

> + =A0linemap_get_statistics (line_table,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &num_ordinary_maps_allo=
cated,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &num_ordinary_maps_used,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &ordinary_maps_allocate=
d_size,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &ordinary_maps_used_siz=
e,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &num_macro_maps_used,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &macro_maps_used_size,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &macro_maps_locations_s=
ize,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &duplicated_maps_locati=
ons_size,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &total_allocated_map_si=
ze,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 &total_used_map_size);

is a too impressive paramater list :-)
Could you use a structure to package all monster?

> +void linemap_get_statistics (struct line_maps *set,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0size_t *, size_t=
 *,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0size_t *, size_t=
 *,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0size_t *, size_t=
 *,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0size_t *, size_t=
 *,
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0size_t *, size_t=
 *);

same here.