Re: [gold][patch] Add more timing information

[Ian Lance Taylor <iant at google dot 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