git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 2/3] grep: stop "using" a custom JIT stack with PCRE v2

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Jul 24, 2019, 15:14 UTC
Message-ID
<20190724151415.3698-3-avarab@gmail.com>
In-Reply-To
<20190721194052.15440-1-carenas@gmail.com>

As reported in [1] the code I added in 94da9193a6 ("grep: add support for PCRE v2", 2017-06-01) to use a custom JIT stack has never worked. It was incorrectly copy/pasted from code I added in fbaceaac47 ("grep: add support for the PCRE v1 JIT API", 2017-05-25), which did work.

Thus our intention of starting with 1 byte of stack at a maximum of 1 MB didn't happen, we'd always use the 32 KB stack provided by PCRE v2's jit_machine_stack_exec()[2]. The reason I allocated a custom stack at all was this advice in pcrejit(3) (same in pcre2jit(3)):

    "By default, it uses 32KiB on the machine stack. However, some
    large or complicated patterns need more than this"

Since we've haven't had any reports of users running into PCRE2_ERROR_JIT_STACKLIMIT in the wild I think we can safely assume that we can just use the library defaults instead and drop this code. This won't change with the wider use of PCRE v2 in ed0479ce3d ("Merge branch 'ab/no-kwset' into next", 2019-07-15), a fixed string search is not a "large or complicated pattern".

For good measure I ran the performance test noted in 94da9193a6, although the command is simpler now due to my 0f50c8e32c ("Makefile: remove the NO_R_TO_GCC_LINKER flag", 2019-05-17):

    GIT_PERF_REPEAT_COUNT=30 GIT_PERF_LARGE_REPO=~/g/linux GIT_PERF_MAKE_OPTS='-j8 USE_LIBPCRE2=Y CFLAGS=-O3 LIBPCREDIR=/home/avar/g/pcre2/inst' ./run HEAD~ HEAD p7820-grep-engines.sh
Just the /perl/ results are:
    Test                                            HEAD~             HEAD
    ---------------------------------------------------------------------------------------
    7820.3: perl grep 'how.to'                      0.17(0.27+0.65)   0.17(0.24+0.68) +0.0%
    7820.7: perl grep '^how to'                     0.16(0.23+0.66)   0.16(0.23+0.67) +0.0%
    7820.11: perl grep '[how] to'                   0.18(0.35+0.62)   0.18(0.33+0.65) +0.0%
    7820.15: perl grep '(e.t[^ ]*|v.ry) rare'       0.17(0.45+0.54)   0.17(0.49+0.50) +0.0%
    7820.19: perl grep 'm(ú|u)lt.b(æ|y)te'          0.16(0.33+0.58)   0.16(0.29+0.62) +0.0%

So, as expected there's no change, and running with valgrind reveals that we have fewer allocations now.

1. https://public-inbox.org/git/20190721194052.15440-1-carenas@gmail.com/
2. I didn't really intend to start with 1 byte, looking at the PCRE v2
   code again what happened is that I cargo-culted some of PCRE v2's
   own test code which was meant to test re-allocations. It's more
   sane to start with say 32 KB with a max of 1 MB, as pcre2grep.c
   does.
Reported-by: Carlo Marcelo Arenas Belón <carenas@gmail.com>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 grep.c | 10 ----------
 grep.h |  4 ----
 2 files changed, 14 deletions(-)
diff --git a/grep.c b/grep.c
index be4282fef3..20ce95270a 100644
--- a/grep.c
+++ b/grep.c
@@ -546,14 +546,6 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
 			p->pcre2_jit_on = 0;
 			return;
 		}
-
-		p->pcre2_jit_stack = pcre2_jit_stack_create(1, 1024 * 1024, NULL);
-		if (!p->pcre2_jit_stack)
-			die("Couldn't allocate PCRE2 JIT stack");
-		p->pcre2_match_context = pcre2_match_context_create(NULL);
-		if (!p->pcre2_match_context)
-			die("Couldn't allocate PCRE2 match context");
-		pcre2_jit_stack_assign(p->pcre2_match_context, NULL, p->pcre2_jit_stack);
 	}
 }
 
@@ -597,8 +589,6 @@ static void free_pcre2_pattern(struct grep_pat *p)
 	pcre2_compile_context_free(p->pcre2_compile_context);
 	pcre2_code_free(p->pcre2_pattern);
 	pcre2_match_data_free(p->pcre2_match_data);
-	pcre2_jit_stack_free(p->pcre2_jit_stack);
-	pcre2_match_context_free(p->pcre2_match_context);
 }
 #else /* !USE_LIBPCRE2 */
 static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)
diff --git a/grep.h b/grep.h
index 1875880f37..a65f4a1ae1 100644
--- a/grep.h
+++ b/grep.h
@@ -29,8 +29,6 @@ typedef int pcre_jit_stack;
 typedef int pcre2_code;
 typedef int pcre2_match_data;
 typedef int pcre2_compile_context;
-typedef int pcre2_match_context;
-typedef int pcre2_jit_stack;
 #endif
 #include "kwset.h"
 #include "thread-utils.h"
@@ -94,8 +92,6 @@ struct grep_pat {
 	pcre2_code *pcre2_pattern;
 	pcre2_match_data *pcre2_match_data;
 	pcre2_compile_context *pcre2_compile_context;
-	pcre2_match_context *pcre2_match_context;
-	pcre2_jit_stack *pcre2_jit_stack;
 	uint32_t pcre2_jit_on;
 	kwset_t kws;
 	unsigned fixed:1;
-- 
2.22.0.455.g172b71a6c5
Previous: Johannes SixtNext: Junio C Hamano
Message 58 of 70 in “grep: use custom JIT stack with pcre2”
  1. grep: use custom JIT stack with pcre2Carlo Marcelo Arenas Belón, Jul 21, 2019
  2. 0/3 grep: PCRE JIT fixesÆvar Arnfjörð Bjarmason, Jul 24, 2019
  3. Junio C HamanoJul 24, 2019
  4. Ævar Arnfjörð BjarmasonJul 24, 2019
  5. 1/8 grep: remove overly paranoid BUG(...) codeÆvar Arnfjörð Bjarmason, Jul 26, 2019
  6. 2/8 grep: stop "using" a custom JIT stack with PCRE v2Ævar Arnfjörð Bjarmason, Jul 26, 2019
  7. Carlo ArenasJul 29, 2019
  8. 3/8 grep: stop using a custom JIT stack with PCRE v1Ævar Arnfjörð Bjarmason, Jul 26, 2019
  9. Carlo ArenasJul 29, 2019
  10. 4/8 grep: consistently use "p->fixed" in compile_regexp()Ævar Arnfjörð Bjarmason, Jul 26, 2019
  11. Carlo ArenasJul 29, 2019
  12. Ævar Arnfjörð BjarmasonJul 29, 2019
  13. Ævar Arnfjörð BjarmasonJul 29, 2019
  14. Junio C HamanoJul 29, 2019
  15. 5/8 grep: create a "is_fixed" member in "grep_pat"Ævar Arnfjörð Bjarmason, Jul 26, 2019
  16. 7/8 grep: do not enter PCRE2_UTF mode on fixed matchingÆvar Arnfjörð Bjarmason, Jul 26, 2019
  17. Junio C HamanoJul 26, 2019
  18. 6/8 grep: stess test PCRE v2 on invalid UTF-8 dataÆvar Arnfjörð Bjarmason, Jul 26, 2019
  19. Junio C HamanoJul 26, 2019
  20. Ævar Arnfjörð BjarmasonJul 26, 2019
  21. Carlo ArenasJul 29, 2019
  22. t7812: add missing redirectsAndreas Schwab, Nov 26, 2019
  23. Johannes SchindelinNov 26, 2019
  24. Andreas SchwabNov 26, 2019
  25. Jeff KingNov 27, 2019
  26. t7812: expect failure for grep -i with invalid UTF-8 dataTodd Zullinger, Nov 30, 2019
  27. Andreas SchwabNov 30, 2019
  28. Junio C HamanoDec 1, 2019
  29. Andreas SchwabDec 1, 2019
  30. Todd ZullingerDec 1, 2019
  31. Junio C HamanoDec 2, 2019
  32. 0/8 grep: PCRE JIT fixes + ab/no-kwset fixÆvar Arnfjörð Bjarmason, Jul 26, 2019
  33. Junio C HamanoJul 26, 2019
  34. Ævar Arnfjörð BjarmasonJul 29, 2019
  35. Junio C HamanoJul 29, 2019
  36. 8/8 grep: optimistically use PCRE2_MATCH_INVALID_UTFÆvar Arnfjörð Bjarmason, Jul 26, 2019
  37. Junio C HamanoJul 26, 2019
  38. Ævar Arnfjörð BjarmasonJul 26, 2019
  39. Ævar Arnfjörð BjarmasonJul 26, 2019
  40. 0/4 grep: better support invalid UTF-8 haystacksÆvar Arnfjörð Bjarmason, Jan 24, 2021
  41. 1/2 grep/pcre2 tests: don't rely on invalid UTF-8 data testÆvar Arnfjörð Bjarmason, Jan 24, 2021
  42. 2/2 grep/pcre2: better support invalid UTF-8 haystacksÆvar Arnfjörð Bjarmason, Jan 24, 2021
  43. Ramsay JonesJan 24, 2021
  44. Ramsay JonesJan 24, 2021
  45. Ævar Arnfjörð BjarmasonJan 24, 2021
  46. Ramsay JonesJan 24, 2021
  47. Ævar Arnfjörð BjarmasonJan 24, 2021
  48. 0/2 grep: better support invalid UTF-8 haystacksÆvar Arnfjörð Bjarmason, Jan 24, 2021
  49. 0/2 grep: better support invalid UTF-8 haystacksÆvar Arnfjörð Bjarmason, Jan 24, 2021
  50. 1/2 grep/pcre2 tests: don't rely on invalid UTF-8 data testÆvar Arnfjörð Bjarmason, Jan 24, 2021
  51. 2/2 grep/pcre2: better support invalid UTF-8 haystacksÆvar Arnfjörð Bjarmason, Jan 24, 2021
  52. 1/4 grep/pcre2 tests: don't rely on invalid UTF-8 data testÆvar Arnfjörð Bjarmason, Jan 24, 2021
  53. 4/4 grep/pcre2: better support invalid UTF-8 haystacksÆvar Arnfjörð Bjarmason, Jan 24, 2021
  54. 3/4 grep/pcre2: further simplify boolean spaghettiÆvar Arnfjörð Bjarmason, Jan 24, 2021
  55. 2/4 grep/pcre2: simplify boolean spaghettiÆvar Arnfjörð Bjarmason, Jan 24, 2021
  56. Junio C HamanoJan 24, 2021
  57. Johannes SixtJan 24, 2021
  58. 2/3 grep: stop "using" a custom JIT stack with PCRE v2Ævar Arnfjörð Bjarmason, Jul 24, 2019
  59. Junio C HamanoJul 24, 2019
  60. Ævar Arnfjörð BjarmasonJul 24, 2019
  61. Carlo ArenasJul 25, 2019
  62. 1/3 grep: remove overly paranoid BUG(...) codeÆvar Arnfjörð Bjarmason, Jul 24, 2019
  63. 3/3 grep: stop using a custom JIT stack with PCRE v1Ævar Arnfjörð Bjarmason, Jul 24, 2019
  64. Carlo ArenasJul 26, 2019
  65. Ævar Arnfjörð BjarmasonJul 26, 2019
  66. Carlo ArenasJul 26, 2019
  67. Ævar Arnfjörð BjarmasonJul 26, 2019
  68. 0/2 PCRE1 cleanupCarlo Marcelo Arenas Belón, Jul 26, 2019
  69. 1/2 grep: make sure NO_LIBPCRE1_JIT disable JIT in PCRE1Carlo Marcelo Arenas Belón, Jul 26, 2019
  70. 2/2 grep: refactor and simplify PCRE1 supportCarlo Marcelo Arenas Belón, Jul 26, 2019

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.