Skip to content

Commit cbe81e6

Browse files
avargitster
authored andcommitted
grep/pcre2: move back to thread-only PCREv2 structures
Change the setup of the "pcre2_general_context" to happen per-thread in compile_pcre2_pattern() instead of in grep_init(). This change brings it in line with how the rest of the pcre2_* members in the grep_pat structure are set up. As noted in the preceding commit the approach 513f2b0 (grep: make PCRE2 aware of custom allocator, 2019-10-16) took to allocate the pcre2_general_context seems to have been initially based on a misunderstanding of how PCREv2 memory allocation works. The approach of creating a global context in grep_init() is just added complexity for almost zero gain. On my system it's 24 bytes saved per-thread. For comparison PCREv2 will then go on to allocate at least a kilobyte for its own thread-local state. As noted in 6d423dd (grep: don't redundantly compile throwaway patterns under threading, 2017-05-25) the grep code is intentionally not trying to micro-optimize allocations by e.g. sharing some PCREv2 structures globally, while making others thread-local. So let's remove this special case and make all of them thread-local again for simplicity. With this change we could move the pcre2_{malloc,free} functions around to live closer to their current use. I'm not doing that here to keep this change small, that cleanup will be done in a follow-up commit. See also the discussion in 94da919 (grep: add support for PCRE v2, 2017-06-01) about thread safety, and Johannes's comments[1] to the effect that we should be doing what this patch is doing. 1. https://lore.kernel.org/git/[email protected]/ Signed-off-by: Ævar Arnfjörð Bjarmason <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 8d12851 commit cbe81e6

File tree

3 files changed

+17
-28
lines changed

3 files changed

+17
-28
lines changed

builtin/grep.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1175,6 +1175,5 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
11751175
run_pager(&opt, prefix);
11761176
clear_pathspec(&pathspec);
11771177
free_grep_patterns(&opt);
1178-
grep_destroy();
11791178
return !hit;
11801179
}

grep.c

Lines changed: 15 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ static struct grep_opt grep_defaults = {
4141
};
4242

4343
#ifdef USE_LIBPCRE2
44-
static pcre2_general_context *pcre2_global_context;
4544
#define GREP_PCRE2_DEBUG_MALLOC 0
4645

4746
static void *pcre2_malloc(PCRE2_SIZE size, MAYBE_UNUSED void *memory_data)
@@ -163,20 +162,9 @@ int grep_config(const char *var, const char *value, void *cb)
163162
* Initialize one instance of grep_opt and copy the
164163
* default values from the template we read the configuration
165164
* information in an earlier call to git_config(grep_config).
166-
*
167-
* If using PCRE, make sure that the library is configured
168-
* to use the same allocator as Git (e.g. nedmalloc on Windows).
169-
*
170-
* Any allocated memory needs to be released in grep_destroy().
171165
*/
172166
void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix)
173167
{
174-
#if defined(USE_LIBPCRE2)
175-
if (!pcre2_global_context)
176-
pcre2_global_context = pcre2_general_context_create(
177-
pcre2_malloc, pcre2_free, NULL);
178-
#endif
179-
180168
*opt = grep_defaults;
181169

182170
opt->repo = repo;
@@ -186,13 +174,6 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix
186174
opt->header_tail = &opt->header_list;
187175
}
188176

189-
void grep_destroy(void)
190-
{
191-
#ifdef USE_LIBPCRE2
192-
pcre2_general_context_free(pcre2_global_context);
193-
#endif
194-
}
195-
196177
static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct grep_opt *opt)
197178
{
198179
/*
@@ -384,13 +365,20 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
384365
int patinforet;
385366
size_t jitsizearg;
386367

387-
/* pcre2_global_context is initialized in grep_init */
368+
/*
369+
* Call pcre2_general_context_create() before calling any
370+
* other pcre2_*(). It sets up our malloc()/free() functions
371+
* with which everything else is allocated.
372+
*/
373+
p->pcre2_general_context = pcre2_general_context_create(
374+
pcre2_malloc, pcre2_free, NULL);
375+
if (!p->pcre2_general_context)
376+
die("Couldn't allocate PCRE2 general context");
377+
388378
if (opt->ignore_case) {
389379
if (!opt->ignore_locale && has_non_ascii(p->pattern)) {
390-
if (!pcre2_global_context)
391-
BUG("pcre2_global_context uninitialized");
392-
p->pcre2_tables = pcre2_maketables(pcre2_global_context);
393-
p->pcre2_compile_context = pcre2_compile_context_create(pcre2_global_context);
380+
p->pcre2_tables = pcre2_maketables(p->pcre2_general_context);
381+
p->pcre2_compile_context = pcre2_compile_context_create(p->pcre2_general_context);
394382
pcre2_set_character_tables(p->pcre2_compile_context,
395383
p->pcre2_tables);
396384
}
@@ -411,7 +399,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt
411399
p->pcre2_compile_context);
412400

413401
if (p->pcre2_pattern) {
414-
p->pcre2_match_data = pcre2_match_data_create_from_pattern(p->pcre2_pattern, pcre2_global_context);
402+
p->pcre2_match_data = pcre2_match_data_create_from_pattern(p->pcre2_pattern, p->pcre2_general_context);
415403
if (!p->pcre2_match_data)
416404
die("Couldn't allocate PCRE2 match data");
417405
} else {
@@ -491,10 +479,11 @@ static void free_pcre2_pattern(struct grep_pat *p)
491479
pcre2_code_free(p->pcre2_pattern);
492480
pcre2_match_data_free(p->pcre2_match_data);
493481
#ifdef GIT_PCRE2_VERSION_10_34_OR_HIGHER
494-
pcre2_maketables_free(pcre2_global_context, p->pcre2_tables);
482+
pcre2_maketables_free(p->pcre2_general_context, p->pcre2_tables);
495483
#else
496484
free((void *)p->pcre2_tables);
497485
#endif
486+
pcre2_general_context_free(p->pcre2_general_context);
498487
}
499488
#else /* !USE_LIBPCRE2 */
500489
static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)

grep.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
typedef int pcre2_code;
1515
typedef int pcre2_match_data;
1616
typedef int pcre2_compile_context;
17+
typedef int pcre2_general_context;
1718
#endif
1819
#ifndef PCRE2_MATCH_INVALID_UTF
1920
/* PCRE2_MATCH_* dummy also with !USE_LIBPCRE2, for test-pcre2-config.c */
@@ -75,6 +76,7 @@ struct grep_pat {
7576
pcre2_code *pcre2_pattern;
7677
pcre2_match_data *pcre2_match_data;
7778
pcre2_compile_context *pcre2_compile_context;
79+
pcre2_general_context *pcre2_general_context;
7880
const uint8_t *pcre2_tables;
7981
uint32_t pcre2_jit_on;
8082
unsigned fixed:1;
@@ -167,7 +169,6 @@ struct grep_opt {
167169

168170
int grep_config(const char *var, const char *value, void *);
169171
void grep_init(struct grep_opt *, struct repository *repo, const char *prefix);
170-
void grep_destroy(void);
171172
void grep_commit_pattern_type(enum grep_pattern_type, struct grep_opt *opt);
172173

173174
void append_grep_pat(struct grep_opt *opt, const char *pat, size_t patlen, const char *origin, int no, enum grep_pat_token t);

0 commit comments

Comments
 (0)