[quake3-bugzilla] [Bug 3639] BoxOnPlaneSide patch

[bugzilla-daemon at icculus.org]

http://bugzilla.icculus.org/show_bug.cgi?id=3639

Matt Turner <mattst88 at gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |mattst88 at gmail.com

Tim Angus <tim at ngus.net> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |tim at ngus.net

--- Comment #5 from Matt Turner <mattst88 at gmail.com> 2009-09-16 23:05:45 EDT ---
I think the really nice thing about this patch is that it replaces 559
lines of unreadable x86 FPU assembly code with _nine_ lines of C that
are at least as fast, and maybe trivially faster. I think it's a no
brainer.

See for yourself. Save the patch and run

egrep -c '^\-[^-]' the.patch

to see how many lines of code were removed, and

egrep -c '^\+[^+]' the.patch

to see how many lines were added.

--- Comment #6 from Tim Angus <tim at ngus.net> 2009-09-17 05:32:24 EDT ---
(In reply to comment #5)
> I think the really nice thing about this patch is that it replaces 559
> lines of unreadable x86 FPU assembly code with _nine_ lines of C that
> are at least as fast, and maybe trivially faster.

Correct me if I'm wrong, but it looks to me like the performance tests are
comparing the old C version with the new C version, not the old x86 with the
new C version? Assuming this is the case, it may not be faster.

In any case, this doesn't really matter; the cleaner code is probably
justification in itself. Having said that, let me echo Ryan's comments; changes
to fundamental math functions need to be definitely correct. Even trivially
small differences in output can result in problems (prediction misses) as we
must cooperate with legacy clients/servers which may use the old code.

So rather than tests for performance, tests for correctness are really what is
needed here.

-- 
Configure bugmail: http://bugzilla.icculus.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.