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@xxxxxxxxx/
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@xxxxxxxxx>
Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@xxxxxxxxx>
---
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
