Bug 165471 - bsdgrep(1) hang/very slow with mmap
Summary: bsdgrep(1) hang/very slow with mmap
Status: Closed FIXED
Alias: None
Product: Base System
Classification: Unclassified
Component: bin (show other bugs)
Version: Unspecified
Hardware: Any Any
: Normal Affects Only Me
Assignee: Kyle Evans
URL:
Keywords: patch
Depends on:
Blocks:
 
Reported: 2012-02-25 14:40 UTC by patdung100
Modified: 2017-08-24 14:17 UTC (History)
4 users (show)

See Also:
emaste: mfc-stable11?
emaste: mfc-stable10-
emaste: mfc-stable9-


Attachments
Proposed patch to address issues with bsdgrep --mmap (820 bytes, patch)
2017-01-20 04:59 UTC, Kyle Evans
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description patdung100 2012-02-25 14:40:14 UTC
bsdgrep with --mmap would sometimes cause hang/slow response.
Also it is eating memory when it is very slow/hang.

cd /usr/local/share and compare grep and bsdgrep:

[root@fbsd9-test /usr/local/share]# time grep --mmap -r -e '^Feb' *
deplate/locale/de.latin1:February
deplate/locale/de.latin1:Februar

real    0m1.875s
user    0m0.085s
sys     0m1.327s
[root@fbsd9-test /usr/local/share]# time bsdgrep --mmap -r -e '^Feb' *
^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C

real    1m40.931s
user    0m0.427s
sys     1m22.064s

Fix: 

use GNU grep
How-To-Repeat: only encounter problem with some files
Comment 1 patdung100 2013-09-15 08:58:48 UTC
After one and a half year, I have tried the same case on FreeBSD 9.2 RC4 x64.
The result is as below. The mmap with bsdgrep option is eating memory/swap and it is killed due to out of swap.


[root@server2 /usr/local/share]# time grep --mmap -r -e '^Feb' *

real    0m14.200s
user    0m0.007s
sys     0m0.932s
[root@server2 /usr/local/share]# time bsdgrep --mmap -r -e '^Feb' *
Killed: 9

real    1m0.474s
user    0m11.218s
sys     0m22.911s

swap_pager_getswapspace(16): failed
swap_pager_getswapspace(16): failed
swap_pager_getswapspace(16): failed
pid 3539 (bsdgrep), uid 0, was killed: out of swap space
Comment 2 patdung100 2014-07-17 07:13:53 UTC
Ten months later, on FreeBSD 9.3 x64, the problem persists.

[root@server3 /usr/local/share]# time grep --mmap -r -e '^October' *
doc/db42/ref/refs/bdb_usenix.html:October 1980.

real    0m0.290s
user    0m0.177s
sys     0m0.113s

I have to press Ctrl-C, and it consumed lots of CPU and RAM.
[root@server3 /usr/local/share]# time bsdgrep --mmap -r -e '^October' *
^C

real    1m20.578s
user    0m9.588s
sys     0m27.200s
Comment 3 Mateusz Guzik freebsd_committer freebsd_triage 2014-07-17 11:48:28 UTC
bsdgrep --mmap does not handle files without newlines at the end properly

issue can be reproduced with:
$ echo -n foo > file
$ bsdgrep --mmap meh file

I don't know this code and I'm not interested in changing it, but the following hack fixes:
diff --git a/usr.bin/grep/file.c b/usr.bin/grep/file.c
index 6bcaa52..f056697 100644
--- a/usr.bin/grep/file.c
+++ b/usr.bin/grep/file.c
@@ -83,12 +83,12 @@ grep_refill(struct file *f)
 {
        ssize_t nr;
 
-       if (filebehave == FILE_MMAP)
-               return (0);
-
        bufpos = buffer;
        bufrem = 0;
 
+       if (filebehave == FILE_MMAP)
+               return (0);
+
        if (filebehave == FILE_GZIP) {
                nr = gzread(gzbufdesc, b
Comment 4 Kyle Evans freebsd_committer freebsd_triage 2017-01-20 04:59:42 UTC
Created attachment 179123 [details]
Proposed patch to address issues with bsdgrep --mmap

As pointed out by Mateusz, bsdgrep --mmap does not handle EOF properly when the file does not end with a newline. I still don't get quite the performance in /usr/local/share that the original reporter does, but bsdgrep is now actually responsive in /usr/local/share.

I'm attaching a patch that reworks bits of the loop in grep_fgetln to return the rest of the line and ensure that we still advance the buffer by the length of the rest of the line. This improves both the original scenario as well as Mateusz's more trivial test case.
Comment 5 commit-hook freebsd_committer freebsd_triage 2017-02-19 17:23:39 UTC
A commit references this bug:

Author: emaste
Date: Sun Feb 19 17:23:28 UTC 2017
New revision: 313948
URL: https://svnweb.freebsd.org/changeset/base/313948

Log:
  bsdgrep: fix EOF handling with --mmap

  Rework part of the loop in grep_fgetln to return the rest of the line
  and ensure that we still advance the buffer by the length of the rest
  of the line.

  PR:		165471
  Submitted by:	Kyle Evans <kevans91@ksu.edu>
  MFC after:	1 month

Changes:
  head/usr.bin/grep/file.c
Comment 6 Ed Maste freebsd_committer freebsd_triage 2017-02-19 17:26:35 UTC
Kyle: I changed wrapping & one comment to maintain an 80-col line length. Thanks for the submission.

(I see further tweaks in your standalone repo; I'll update with those later.)
Comment 7 Ed Maste freebsd_committer freebsd_triage 2017-05-19 13:45:37 UTC
I just tried a similar test with bsdgrep to reconfirm, and it aborted. Details in PR 219402
Comment 8 Ed Maste freebsd_committer freebsd_triage 2017-05-20 00:37:13 UTC
With the fix from PR219402 applied I see:

   grep --mmap -r -e '^Feb' *  0.43s user 1.53s system 94% cpu 2.080 total
bsdgrep --mmap -r -e '^Feb' *  0.56s user 2.24s system 98% cpu 2.838 total
Comment 9 commit-hook freebsd_committer freebsd_triage 2017-08-14 21:48:57 UTC
A commit references this bug:

Author: kevans
Date: Mon Aug 14 21:48:50 UTC 2017
New revision: 322520
URL: https://svnweb.freebsd.org/changeset/base/322520

Log:
  MFC r313948: bsdgrep: fix EOF handling with --mmap

  Rework part of the loop in grep_fgetln to return the rest of the line
  and ensure that we still advance the buffer by the length of the rest
  of the line.

  PR:		165471
  Approved by:	emaste (mentor)

Changes:
_U  stable/11/
  stable/11/usr.bin/grep/file.c
Comment 10 Mark Linimon freebsd_committer freebsd_triage 2017-08-24 14:17:28 UTC
MFCed Mon Aug 14 21:48:50 UTC 2017.