@matheusbsbs / matheus-bagliano / commits / cbe81e653f

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 513f2b0bbd4 (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 6d423dd542f (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 94da9193a6 (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/nycvar.QRO.7.76.6.1908052120302.46@tvgsbejvaqbjf.bet/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Feb 18, 2021 at 01:07 UTC cbe81e653fa1adc6b4e09d881628074f7448289a

3 files changed +17 -28

builtin/grep.c

-1

@@ -1175,6 +1175,5 @@ int cmd_grep(int argc, const char **argv, const char *prefix)

1175 run_pager(&opt, prefix);

1176 clear_pathspec(&pathspec);

1177 free_grep_patterns(&opt);

1178 - grep_destroy();

1178 return !hit;

1179 }

grep.c

+15 -26

@@ -41,7 +41,6 @@ static struct grep_opt grep_defaults = {

41 };

43 #ifdef USE_LIBPCRE2

44 -static pcre2_general_context *pcre2_global_context;

44 #define GREP_PCRE2_DEBUG_MALLOC 0

46 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)

162 * Initialize one instance of grep_opt and copy the

163 * default values from the template we read the configuration

164 * 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().

165 */

166 void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix)

167 {

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 -

168 *opt = grep_defaults;

169

170 opt->repo = repo;

@@ -186,13 +174,6 @@ void grep_init(struct grep_opt *opt, struct repository *repo, const char *prefix

174 opt->header_tail = &opt->header_list;

175 }

176

189 -void grep_destroy(void)

190 -{

191 -#ifdef USE_LIBPCRE2

192 - pcre2_general_context_free(pcre2_global_context);

193 -#endif

194 -}

195 -

177 static void grep_set_pattern_type_option(enum grep_pattern_type pattern_type, struct grep_opt *opt)

178 {

179 /*

@@ -384,13 +365,20 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt

365 int patinforet;

366 size_t jitsizearg;

367

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 +

378 if (opt->ignore_case) {

379 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);

382 pcre2_set_character_tables(p->pcre2_compile_context,

383 p->pcre2_tables);

384 }

@@ -411,7 +399,7 @@ static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt

399 p->pcre2_compile_context);

400

401 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);

403 if (!p->pcre2_match_data)

404 die("Couldn't allocate PCRE2 match data");

405 } else {

@@ -491,10 +479,11 @@ static void free_pcre2_pattern(struct grep_pat *p)

479 pcre2_code_free(p->pcre2_pattern);

480 pcre2_match_data_free(p->pcre2_match_data);

481 #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);

483 #else

484 free((void *)p->pcre2_tables);

485 #endif

486 + pcre2_general_context_free(p->pcre2_general_context);

487 }

488 #else /* !USE_LIBPCRE2 */

489 static void compile_pcre2_pattern(struct grep_pat *p, const struct grep_opt *opt)

grep.h

+2 -1

@@ -14,6 +14,7 @@

14 typedef int pcre2_code;

15 typedef int pcre2_match_data;

16 typedef int pcre2_compile_context;

17 +typedef int pcre2_general_context;

18 #endif

19 #ifndef PCRE2_MATCH_INVALID_UTF

20 /* PCRE2_MATCH_* dummy also with !USE_LIBPCRE2, for test-pcre2-config.c */

@@ -75,6 +76,7 @@ struct grep_pat {

76 pcre2_code *pcre2_pattern;

77 pcre2_match_data *pcre2_match_data;

78 pcre2_compile_context *pcre2_compile_context;

79 + pcre2_general_context *pcre2_general_context;

80 const uint8_t *pcre2_tables;

81 uint32_t pcre2_jit_on;

82 unsigned fixed:1;

@@ -167,7 +169,6 @@ struct grep_opt {

169

170 int grep_config(const char *var, const char *value, void *);

171 void grep_init(struct grep_opt *, struct repository *repo, const char *prefix);

170 -void grep_destroy(void);

172 void grep_commit_pattern_type(enum grep_pattern_type, struct grep_opt *opt);

173

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