[MPlayer-dev-eng] [PATCH] revert CPU speed detection

[Diego Biurrun]

D Richard Felker III writes:
> On Thu, Oct 07, 2004 at 11:31:47AM +0200, Diego Biurrun wrote:
> > D Richard Felker III writes:
> > > On Thu, Oct 07, 2004 at 04:34:50AM +0200, Diego Biurrun wrote:
> > > > 
> > > > I've had a look at a way to make the CPU speed detection run only in
> > > > verbose mode but then reconsidered.  Speed detection has never worked
> > > > reliably and slows down startup noticeably.  Thus I propose the
> > > > following patch that rips out CPU speed detection by reverting the
> > > > commit from Atmos that added it:
> > > > 
> > > > http://mplayerhq.hu/pipermail/mplayer-cvslog/2003-September/016619.html
> > > > 
> > > > What do you think?
> > > 
> > > i'm against applying this patch as-is. it's just a reversed version of
> > > the original patches. granted those seem to have had some cosmetics,
> > > but reverting that now is just more cosmetics.
> > 
> > Not reverting the cosmetics defeats the purpose of forbidding
> > cosmetics in the first place.  No cosmetics lead to cleaner CVS diffs,
> > if you keep the cosmetics now, the diffs remain unreadable, so what
> > are we supposed to gain by not removing the cosmetics?
> 
> right now we have one unreadable diff. if we commit your patch we have
> two unreadable diffs. 2>1 :)

My original idea was that committing an inverse patch would "cancel
out" the cpuspeed detection commit across cvs diffs.

Let me explain: Let's assume (actual numbers are not important) that
cpuspeed was committed as version 1.10 of cpudetect.c and the inverse
patch is committed as version 1.13.  If you are at revision 1.20 and
wish to look at cvs diff between, say 1.8 and 1.15 you will look at a
diff that contains only the changes from 1.9, 1.11, 1.12, 1.14, 1.15.
This should be easier to understand than having 1.10 partially still
in there.

See my point?

Also I prefer splitting up commits to small self-contained units, so I
think

1.10: cpuspeed detection containing useful feature XXX
1.11
1.12
1.13: back out cpuspeed detection completely
1.14: useful feature XXX

is cleaner than

1.10: cpuspeed detection containing useful feature XXX
1.11
1.12
1.13: back out cpuspeed detection except for useful feature XXX

But all of this may be personal preference and not overly important...

> > I'm not against cherry picking the good parts out of the speed
> > detection code.  IMO this should be done in a separate patch, though.
> > That patch will be clean and easy to verify just as this one.  This
> > would also separate each logical step into one commit as it should be
> > done.  One cleanly backs out speed detection, one small patch adds TSC
> > detection.
> 
> i'm VERY MUCH against committing one patch that removes hasTSC from
> cpudetect.h and then another that adds it back, since this is the only
> change made to cpudetect.h! it's just cvs pollution.

It allows telling to which commit it belongs.  It would be much more
useful if CVS had support for real changesets instead of registering
changes file by file...

But this, too, is a matter of personal preference and not overly
important..

Diego