Re: grep: convert fgetln to getline

2019-01-31 Thread Gilles Chehade
On Thu, Jan 31, 2019 at 03:10:53PM +0200, Lauri Tirkkonen wrote: > On Wed, Jan 30 2019 20:32:50 -0500, Ted Unangst wrote: > > Thanks for digging into this. I went ahead and committed your diff. > > Thanks for committing it. > > You know, having seen fgetln's allocation strategy and its usage of

Re: grep: convert fgetln to getline

2019-01-31 Thread Lauri Tirkkonen
On Wed, Jan 30 2019 20:32:50 -0500, Ted Unangst wrote: > Thanks for digging into this. I went ahead and committed your diff. Thanks for committing it. You know, having seen fgetln's allocation strategy and its usage of the stdio internals, I couldn't help but wonder if it's something that could

Re: grep: convert fgetln to getline

2019-01-30 Thread Ted Unangst
Lauri Tirkkonen wrote: > In summary: sorry for the bug I introduced - please apply the diff above > to fix memory mapping files in grep. As for the fgetln->getline > conversion, I think this result should show that fgetln is not > particularly smart about how it's calling mmap -- getline seems

Re: grep: convert fgetln to getline

2019-01-27 Thread Lauri Tirkkonen
On Thu, Jan 24 2019 17:03:57 -0700, Theo de Raadt wrote: > Scott Cheloha wrote: > > > > On Jan 24, 2019, at 06:19, Lauri Tirkkonen wrote: > > > > > > [...] > > > > > > I haven't done any actual measurements though, so it's possible my > > > reading is wrong. > > > > Is there a "grepbench"

Re: grep: convert fgetln to getline

2019-01-24 Thread Lauri Tirkkonen
On Thu, Jan 24 2019 18:08:25 -0600, Scott Cheloha wrote: > not to be standoffish, I'm just travelling so I can't really look into it. yeah, it's not like I have infinite time either. I'll try to do something with this over the weekend but no promises. > if you came forward with malloc numbers

Re: grep: convert fgetln to getline

2019-01-24 Thread Theo de Raadt
Scott Cheloha wrote: > > On Jan 24, 2019, at 06:19, Lauri Tirkkonen wrote: > > > > [...] > > > > I haven't done any actual measurements though, so it's possible my > > reading is wrong. > > Is there a "grepbench" or canonical corpus we can use to get a 95% > CI on this and future changes?

Re: grep: convert fgetln to getline

2019-01-24 Thread Scott Cheloha
> On Jan 24, 2019, at 06:19, Lauri Tirkkonen wrote: > > [...] > > I haven't done any actual measurements though, so it's possible my > reading is wrong. Is there a "grepbench" or canonical corpus we can use to get a 95% CI on this and future changes? People talk about grep(1) perf. like CPU

Re: grep: convert fgetln to getline

2019-01-24 Thread Lauri Tirkkonen
On Thu, Jan 24 2019 13:58:04 +0200, Lauri Tirkkonen wrote: > And even then I think the cost is negligible: getline grows > the buffer in powers of 2, so only lines that happen to be twice as long > as any previously encountered line pay the price. Actually, I went to skim the fgetln()

Re: grep: convert fgetln to getline

2019-01-24 Thread Lauri Tirkkonen
On Thu, Jan 24 2019 04:40:08 -0700, Theo de Raadt wrote: > >On Thu, Jan 24 2019 04:22:20 -0700, Theo de Raadt wrote: > >> I would like to know if this does more malloc. I worry it is an additional > >> level of malloc per line. > > > >It does do more malloc than plain fgetln since fgetln does no

Re: grep: convert fgetln to getline

2019-01-24 Thread Theo de Raadt
>On Thu, Jan 24 2019 04:22:20 -0700, Theo de Raadt wrote: >> I would like to know if this does more malloc. I worry it is an additional >> level of malloc per line. > >It does do more malloc than plain fgetln since fgetln does no copying, >but nowhere near every line. The same buffer lnbuf is

Re: grep: convert fgetln to getline

2019-01-24 Thread Lauri Tirkkonen
On Thu, Jan 24 2019 04:22:20 -0700, Theo de Raadt wrote: > I would like to know if this does more malloc. I worry it is an additional > level of malloc per line. It does do more malloc than plain fgetln since fgetln does no copying, but nowhere near every line. The same buffer lnbuf is used for

Re: grep: convert fgetln to getline

2019-01-24 Thread Theo de Raadt
I would like to know if this does more malloc. I worry it is an additional level of malloc per line. >On Wed, Jan 23 2019 18:01:24 -0500, Ted Unangst wrote: >> Lauri Tirkkonen wrote: >> > > > oh, interesting. that's sloppy. can you please fix that first, >> > > > separately? >> > > >> > >

Re: grep: convert fgetln to getline

2019-01-23 Thread Lauri Tirkkonen
On Wed, Jan 23 2019 18:01:24 -0500, Ted Unangst wrote: > Lauri Tirkkonen wrote: > > > > oh, interesting. that's sloppy. can you please fix that first, > > > > separately? > > > > > > Sure, here it is. > > > > Could you please take a look at it? It's been a couple weeks. > > yup, sorry, slipped

Re: grep: convert fgetln to getline

2019-01-23 Thread Ted Unangst
Lauri Tirkkonen wrote: > > > oh, interesting. that's sloppy. can you please fix that first, separately? > > > > Sure, here it is. > > Could you please take a look at it? It's been a couple weeks. yup, sorry, slipped by. committed.

Re: grep: convert fgetln to getline

2019-01-23 Thread Lauri Tirkkonen
On Mon, Jan 07 2019 23:01:47 +0200, Lauri Tirkkonen wrote: > On Mon, Jan 07 2019 15:41:53 -0500, Ted Unangst wrote: > > Lauri Tirkkonen wrote: > > > On Sun, Jan 06 2019 14:02:16 -0500, Ted Unangst wrote: > > > > Lauri Tirkkonen wrote: > > > > > Hi, another simple diff converting fgetln usage to

Re: grep: convert fgetln to getline

2019-01-14 Thread Lauri Tirkkonen
On Mon, Jan 07 2019 23:01:47 +0200, Lauri Tirkkonen wrote: > On Mon, Jan 07 2019 15:41:53 -0500, Ted Unangst wrote: > > Lauri Tirkkonen wrote: > > > On Sun, Jan 06 2019 14:02:16 -0500, Ted Unangst wrote: > > > > Lauri Tirkkonen wrote: > > > > > Hi, another simple diff converting fgetln usage to

Re: grep: convert fgetln to getline

2019-01-07 Thread Lauri Tirkkonen
On Mon, Jan 07 2019 15:41:53 -0500, Ted Unangst wrote: > Lauri Tirkkonen wrote: > > On Sun, Jan 06 2019 14:02:16 -0500, Ted Unangst wrote: > > > Lauri Tirkkonen wrote: > > > > Hi, another simple diff converting fgetln usage to getline. > > > > > > > > I also considered converting grep_fgetln to

Re: grep: convert fgetln to getline

2019-01-07 Thread Ted Unangst
Lauri Tirkkonen wrote: > On Sun, Jan 06 2019 14:02:16 -0500, Ted Unangst wrote: > > Lauri Tirkkonen wrote: > > > Hi, another simple diff converting fgetln usage to getline. > > > > > > I also considered converting grep_fgetln to grep_getline, but that would > > > mean that for FILE_MMAP we'd have

Re: grep: convert fgetln to getline

2019-01-07 Thread Lauri Tirkkonen
On Sun, Jan 06 2019 14:02:16 -0500, Ted Unangst wrote: > Lauri Tirkkonen wrote: > > Hi, another simple diff converting fgetln usage to getline. > > > > I also considered converting grep_fgetln to grep_getline, but that would > > mean that for FILE_MMAP we'd have to copy the string. So this diff

Re: grep: convert fgetln to getline

2019-01-06 Thread Ted Unangst
Lauri Tirkkonen wrote: > Hi, another simple diff converting fgetln usage to getline. > > I also considered converting grep_fgetln to grep_getline, but that would > mean that for FILE_MMAP we'd have to copy the string. So this diff keeps > the grep_fgetln interface as is, but avoids using fgetln

grep: convert fgetln to getline

2019-01-06 Thread Lauri Tirkkonen
Hi, another simple diff converting fgetln usage to getline. I also considered converting grep_fgetln to grep_getline, but that would mean that for FILE_MMAP we'd have to copy the string. So this diff keeps the grep_fgetln interface as is, but avoids using fgetln from libc (and adds error handling