Re: [PATCH 6/9] grep: cache userdiff_driver in grep_source
- From
Jeff King <peff@peff.net>
- Date
- Feb 2, 2012, 19:37 UTC
- Message-ID
- <20120202193756.GA9246@sigill.intra.peff.net>
- In-Reply-To
- <7v4nv9xexs.fsf@alter.siamese.dyndns.org>
On Thu, Feb 02, 2012 at 10:34:07AM -0800, Junio C Hamano wrote:
Show 14 quoted lines
> Jeff King <peff@peff.net> writes:
>
> > - grep_attr_lock();
> > - drv = userdiff_find_by_path(gs->name);
> > - grep_attr_unlock();
> > - if (drv && drv->funcname.pattern) {
> > - const struct userdiff_funcname *pe = &drv->funcname;
> > + grep_source_load_driver(gs);
> > + if (gs->driver->funcname.pattern) {
> > + const struct userdiff_funcname *pe = &gs->driver->funcname;
>
> When we load driver, gs->driver gets at least "default" driver, so we no
> longer need to check for drv != NULL as we used to? Is that the reason
> for the slight difference here?Yes, exactly.
We could just leave gs->driver NULL instead of looking up "default", and then use NULL to signal to the calling code that defaults should be used. But NULL is interpreted by grep_source_load_driver as "we did not look up the driver yet", so the common case of "no driver" would mean we accidentally do the lookup multiple times. The diff_filespec code uses the same convention to solve the same problem.
Speaking of which, there was some notion in my mind that a "grep_source" and a "diff_filespec" were very similar objects, and that we could possibly unify the implementations. I decided against that route with this series, as it would have involved pretty heavy refactoring of the diff code to prevent a fairly small amount of code duplication.
-Peff