threads / discuss / 27949

memrchr() implementation

Subject: memrchr() implementation

## tl;dr

3 messages between Jul 29, 2011 and Jul 29, 2011.

replies: 2people: 3as markdown or json

Tay Ray Chuan· Jul 29, 2011, 04:33 UTC · lore

Hi,

I'm writing a patch that requires the use of memrchr(). [1] As far as I know, memrchr() is non-standard.

So what's the best way to get git to use memrchr()?

I was thinking of getting git to use glibc's implementation if glibc is present, if not, use a naive implementation included in git's code base [2].

-- Footnotes: [1] For those interested, it's regarding content trimming to speed out diffs. You can see memrchr() in use at https://github.com/rctay/git/blob/tg%2Fgsoc-diff%2Ftrim-ends/xdiff/xprepare.c#L203

[2] (ISC-style license) http://www.sudo.ws/repos/sudo/file/c1ab4b940980/compat/memrchr.c

-- 
Cheers,
Ray Chuan

Jeff King· Jul 29, 2011, 04:40 UTC · re: Tay Ray Chuan · lore

Re: memrchr() implementation

On Fri, Jul 29, 2011 at 12:33:01PM +0800, Tay Ray Chuan wrote:

> I'm writing a patch that requires the use of memrchr(). [1] As far as
> I know, memrchr() is non-standard.

Yep.

Show 5 quoted lines

> So what's the best way to get git to use memrchr()?
> 
> I was thinking of getting git to use glibc's implementation if glibc
> is present, if not, use a naive implementation included in git's code
> base [2].

Yeah, that's our usual strategy; see how strchrnul is implemented for an example.

Since you are doing this for speed and not ease-of-use, I assume you are benchmarking stock git versus glibc's optimized memrchr. I'd be curious to see a comparison to your naive implementation, too, just to double-check that your attempt at speeding up doesn't make things worse on non-glibc platforms.

-Peff

Jonathan Nieder· Jul 29, 2011, 04:47 UTC · re: Tay Ray Chuan · lore

Re: memrchr() implementation

Hi,

Tay Ray Chuan wrote:

> So what's the best way to get git to use memrchr()?

In my very humble opinion: [1].

> I was thinking of getting git to use glibc's implementation if glibc
> is present, if not, use a naive implementation included in git's code
> base [2].

Sounds reasonable.

> [2] (ISC-style license)
> http://www.sudo.ws/repos/sudo/file/c1ab4b940980/compat/memrchr.c

For something simple as this, I'd prefer to avoid a differently licensed (even ISC-licensed) file.

It might also be worth convincing the folks at http://austingroupbugs.net/ to standardize the interface so there could be optimized implementations of memrchr on more platforms over time.

Thanks, Jonathan

[1] http://thread.gmane.org/gmane.comp.version-control.git/159081/focus=159121