[Top] [All Lists]

Re: [PATCH 02/18] xfstests: Clear out old benchmarking materials

To: Philip White <pwhite@xxxxxxx>
Subject: Re: [PATCH 02/18] xfstests: Clear out old benchmarking materials
From: Dave Chinner <david@xxxxxxxxxxxxx>
Date: Fri, 15 Mar 2013 15:05:31 +1100
Cc: xfs@xxxxxxxxxxx
Delivered-to: xfs@xxxxxxxxxxx
In-reply-to: <20130314130611.4A3C153DEAF1@xxxxxxxxxxxxxxxxxxxxxxxxxx>
References: <20130314130611.4A3C153DEAF1@xxxxxxxxxxxxxxxxxxxxxxxxxx>
User-agent: Mutt/1.5.21 (2010-09-15)
On Thu, Mar 14, 2013 at 06:06:11AM -0700, Philip White wrote:
> From: Phil White <pwhite@xxxxxxx>
> This is a rebasing & resubmit of a dchinner patch.  His comments on the
> original:
> -----------------
> The benchmark framework inside xfstests is basically unused,
> bitrotted and not very useful. If we need benchmarks, lets use a
> real benchmark framework, not xfstests. Kill it to remove
> dependencies on common and common.rc.
> -----------------
> The current benchmark framework has bitrotted, but it is not without value
> in my view.  It should be removed and updated.  In the interest of future
> useability, I have kept the dependencies on common and common.rc.

This makes no sense - you've removed everything but the bench
script, which can't do anything without all the bits you just
removed. But you still want to keep a useless script and it's
dependencies around "just because".

Let go of bench. Re-implement it from scratch rather than try to
shoehorn it into the stub of the old, bitrotted code. IOWs, any new
benchmarking infrastructure needs to abstravct and document each
dependency it adds to the common code - that way we don't end up
with keeping unnecessary dependencies around or trying to shoehorn
new code into abstractions that don't make sense.

FWIW, this comes back to what I said about having a patch series
description - I didn't realise at first that you hadn't completely
removed all of the bench infrastructure because there was no
overview of what you'd done. Hence it's only now that I'm looking
at the patches in detail have I noticed this subtle change you made
to my original patch set....


Dave Chinner

<Prev in Thread] Current Thread [Next in Thread>