Re: [PATCH 05/31] environment: remove namespace_len variable

From: Brandon Williams <bmwill@google.com>
Date: Jun 1, 2017, 21:09 UTC
Message-ID: <20170601210911.GA159608@google.com>
In-Reply-To: <CAGZ79kZF3mk40P1rRvV57X=JWveNoCmdvKmkUO=fo8Ts=BkZrA@mail.gmail.com>

On 06/01, Stefan Beller wrote:

Show 11 quoted lines

> On Wed, May 31, 2017 at 2:43 PM, Brandon Williams <bmwill@google.com> wrote:
> > Use 'skip_prefix' instead of 'starts_with' so that we can drop the need
> > to keep around 'namespace_len'.
> 
> Looks correct.
> 
> Performance wise:
> The strip_namespace function is only called from {receive/upload}-pack
> and http-backend, so all network operations, whose actual operations
> should far outweight the tiny CPU saving that this reverts.
> We should be fine?

Actually there should be zero performance degradation with this change (maybe a very small improvement) because both functions need to do a char by char comparison, just skip_prefix returns a pointer into the string with the prefix already stripped. So no need to do a strlen() calculation on 'namespace' as well as no additon operation adding 'namespace_len' to 'namespaced_ref'.

Show 43 quoted lines

> 
> > Signed-off-by: Brandon Williams <bmwill@google.com>
> > ---
> >  environment.c | 9 ++++-----
> >  1 file changed, 4 insertions(+), 5 deletions(-)
> >
> > diff --git a/environment.c b/environment.c
> > index a73b08f5d..e035f6372 100644
> > --- a/environment.c
> > +++ b/environment.c
> > @@ -98,7 +98,6 @@ char *git_work_tree_cfg;
> >  static char *work_tree;
> >
> >  static const char *namespace;
> > -static size_t namespace_len;
> >
> >  static const char *super_prefix;
> >
> > @@ -190,7 +189,6 @@ void setup_git_env(void)
> >         git_replace_ref_base = xstrdup(replace_ref_base ? replace_ref_base
> >                                                           : "refs/replace/");
> >         namespace = expand_namespace(getenv(GIT_NAMESPACE_ENVIRONMENT));
> > -       namespace_len = strlen(namespace);
> >         shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
> >         if (shallow_file)
> >                 set_alternate_shallow_file(shallow_file, 0);
> > @@ -231,9 +229,10 @@ const char *get_git_namespace(void)
> >
> >  const char *strip_namespace(const char *namespaced_ref)
> >  {
> > -       if (!starts_with(namespaced_ref, get_git_namespace()))
> > -               return NULL;
> > -       return namespaced_ref + namespace_len;
> > +       const char *out;
> > +       if (skip_prefix(namespaced_ref, get_git_namespace(), &out))
> > +               return out;
> > +       return NULL;
> >  }
> >
> >  const char *get_super_prefix(void)
> > --
> > 2.13.0.506.g27d5fe0cd-goog
> >

-- 
Brandon Williams