This is the mail archive of the
binutils@sourceware.org
mailing list for the binutils project.
Re: [gold][patch] Add more timing information
- From: Ian Lance Taylor <iant at google dot com>
- To: Rafael Espindola <espindola at google dot com>
- Cc: Binutils <binutils at sourceware dot org>, Diego Novillo <dnovillo at google dot com>, Neil Vachharajani <nvachhar at google dot com>
- Date: Wed, 16 Dec 2009 16:02:06 -0800
- Subject: Re: [gold][patch] Add more timing information
- References: <38a0d8450912161521x1eeb2f93kf155d592c6c9425d@mail.gmail.com>
Rafael Espindola <espindola@google.com> writes:
> 2009-12-16 Rafael Avila de Espindola <espindola@google.com>
>
> * Makefile.am (CCFILES): Add timer.cc.
> (HFILES): Add timer.h.
> * configure.ac: Check for sysconf and times.
> * main.cc: include timer.h.
> (main): Use Timer instead of get_run_time.
> * timer.cc: New.
> * timer.h: New.
> * workqueue.cc: include timer.h.
> (Workqueue::find_and_run_task):
> Report user, sys and wall time.
> * Makefile.in: Regenerate.
> * config.in: Regenerate.
> * configure: Regenerate.
> +++ b/gold/timer.cc
> @@ -0,0 +1,109 @@
> +// timer.cc -- helper class for time accounting
> +
> +// Copyright 2006, 2007, 2008 Free Software Foundation, Inc.
> +// Written by Rafael Avila de Espindola <espindola@google.com>.
Just copyright 2009 here.
> +// After processing the command line, everything the linker does is
> +// driven from a work queue. This permits us to parallelize the
> +// linker where possible.
This comment is wrong in this file.
> +// sysconf can be slow, so we only call it once.
> +static long ticks_per_sec;
The variable itself needs a comment documenting the value.
> +// Write the current time infortamion.
> +void
> +Timer::get_time (TimeStats *now)
> +{
> +#ifdef HAVE_TIMES
> + tms t;
> + now->wall = (times(&t) * 1000) / ticks_per_sec;
Extra whitespace before '*'?
> +++ b/gold/timer.h
> @@ -0,0 +1,71 @@
> +// timer.h -- helper class for time accounting -*- C++ -*-
> +
> +// Copyright 2006, 2007, 2008 Free Software Foundation, Inc.
> +// Written by Rafael Avila de Espindola <espindola@google.com>.
Just cpoyright 2009.
> +// After processing the command line, everything the linker does is
> +// driven from a work queue. This permits us to parallelize the
> +// linker where possible.
This comment is out of place.
> +
> +#ifndef GOLD_TIMER_H
> +#define GOLD_TIMER_H
> +
> +namespace gold
> +{
> +
> +class Timer
> +{
> + public:
> + // Used to report time statistics. All fields are in milliseconds.
> + struct TimeStats
> + {
> + /* User time in this process. */
> + long user;
> +
> + /* System time in this process. */
> + long sys;
> +
> + /* Wall clock time. */
> + long wall;
> + };
> +
> + // Return the stats since start was called.
> + TimeStats
> + get_elapsed_time ();
No space before parenthesis.
> + // Start couting the time.
> + void
> + start();
> +
> + Timer();
The constructor should be the first method declared, immediately after
TimeStats.
> + private:
> + // This class cannot be copied.
> + Timer(const Timer&);
> + Timer& operator=(const Timer&);
> +
> + // Write the current time infortamion.
> + static void
> + get_time (TimeStats *now);
> + TimeStats start_time_;
> +};
Blank line after get_time. No space before parenthesis. Add comment
for start_time_.
This is OK with those changes.
Thanks.
Ian