Re: [PATCH 1/3] speed up debVersioningSystem::DoCmpVersion a bit
On Sat, Jan 25, 2014 at 03:27:43AM +0100, Jann Horn wrote:
> ---
> apt-pkg/deb/debversion.cc | 37 +++++++++++++++++++++++++------------
> 1 file changed, 25 insertions(+), 12 deletions(-)
>
> diff --git a/apt-pkg/deb/debversion.cc b/apt-pkg/deb/debversion.cc
> index 1405612..6b75e66 100644
> --- a/apt-pkg/deb/debversion.cc
> +++ b/apt-pkg/deb/debversion.cc
> @@ -119,6 +119,27 @@ int debVersioningSystem::CmpFragment(const char *A,const char *AEnd,
> // Shouldnt happen
> return 1;
> }
> +
> +const char *memchr_short(const char *s, int c, const char *e)
> +{
> + const char *p = s;
> + while (p != e) {
> + if (*p == c) return p;
> + p++;
> + }
> + return s;
> +}
> +
> +const char *memrchr_short(const char *s, int c, const char *e)
> +{
> + const char *p = e;
> + while (p != s) {
> + p--;
> + if (*p == c) return p;
> + }
> + return e;
> +}
> +
They should be static. And I don't think it makes sense to change
the third argument from a size_t to a const char*. It's kind of
confusing as it's still a memchr.
I do not believe that this is any faster at all. On what architectures
did you test this? Especially important are amd64, armel, armhf.
> /*}}}*/
> // debVS::CmpVersion - Comparison for versions /*{{{*/
> // ---------------------------------------------------------------------
> @@ -128,12 +149,8 @@ int debVersioningSystem::DoCmpVersion(const char *A,const char *AEnd,
> const char *B,const char *BEnd)
> {
> // Strip off the epoch and compare it
> - const char *lhs = (const char*) memchr(A, ':', AEnd - A);
> - const char *rhs = (const char*) memchr(B, ':', BEnd - B);
> - if (lhs == NULL)
> - lhs = A;
> - if (rhs == NULL)
> - rhs = B;
> + const char *lhs = (const char*) memchr_short(A, ':', AEnd);
> + const char *rhs = (const char*) memchr_short(B, ':', BEnd);
You don't really need the cast here anymore.
--
Julian Andres Klode - Debian Developer, Ubuntu Member
See http://wiki.debian.org/JulianAndresKlode and http://jak-linux.org/.
Please do not top-post if possible.
Reply to: