git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Duy Nguyen <pclouds@gmail.com>
Cc: Git Mailing List <git@vger.kernel.org>,
	Junio C Hamano <gitster@pobox.com>, Jeff King <peff@peff.net>
Subject: Re: [PATCH 0/2] wildmatch precompilation interface
Date: Mon, 26 Feb 2018 13:34:07 +0100	[thread overview]
Message-ID: <87muzwdj5s.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <CACsJy8COegzVZ99PqsfrDq=uPfQewEZKXF5WK3K6ah62QfQV9g@mail.gmail.com>


On Mon, Feb 26 2018, Duy Nguyen jotted:

> On Mon, Feb 26, 2018 at 3:35 AM, Ævar Arnfjörð Bjarmason
> <avarab@gmail.com> wrote:
>> My recently landed wildmatch test series was in preperation for some
>> more major wildmatch changes.
>>
>> Here's another series that prepares for bigger changes in wildmatch,
>> it adds an interface to pre-compile the patterns. Right now there's no
>> point in doing this, and it's harmless since none of the codepaths are
>> that performance sensitive, but down the line this'll save us time as
>> we'll be able to skip re-parsing the pattern each time with a better
>> wildmatch backend.
>
> I don't see any big problem with this, but should this be a standalone
> series? Some changes look harmless now, but I'd rather see the real
> precompile implementation to see how it impacts (or benefits) the
> converted call sites.

Let's drop this for now sinceyou're on the fence about it, I wasn't all
that sure myself and thought I'd send it in for comments.

I don't have anything ready for submission from the rest of this series,
but figured (if you/others didn't mind) that it might be easier to
review the addition of the interface at first, but indeed, on second
thought that doesn't make sense.

The state of what I have is:

 1. <this tiny series>
 2. <WIP add the wildmatch compile interface to more stuff, notably dir.c>

    The dir.c use (and some tree-related stuff) are the real hot
    codepaths using wildmatch.
 3. <WIP Replace the wildmatch backend with syntax-compatible PCRE>

    This is somewhat of a mess right now. Preliminary tests reveal that
    the pathological case tested for by t/perf/p0100-globbing.sh is
    wildly faster, but that most regular matching is a bit slower,
    although that might be me being stupid with the interface.

      reply	other threads:[~2018-02-26 12:34 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-25 20:35 [PATCH 0/2] wildmatch precompilation interface Ævar Arnfjörð Bjarmason
2018-02-25 20:35 ` [PATCH 1/2] wildmatch: add interface for precompiling wildmatch() patterns Ævar Arnfjörð Bjarmason
2018-02-26 10:53   ` Duy Nguyen
2018-02-26 12:19     ` Ævar Arnfjörð Bjarmason
2018-02-25 20:35 ` [PATCH 2/2] wildmatch: use the new precompiling wildmatch() Ævar Arnfjörð Bjarmason
2018-02-26 11:00   ` Duy Nguyen
2018-02-26 12:17     ` Ævar Arnfjörð Bjarmason
2018-02-26 11:01 ` [PATCH 0/2] wildmatch precompilation interface Duy Nguyen
2018-02-26 12:34   ` Ævar Arnfjörð Bjarmason [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87muzwdj5s.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).