bryancall commented on PR #13683: URL: https://github.com/apache/trafficserver/pull/13683#issuecomment-6622815390
@JosiahWI Thanks for making time for this. To help split it up, here is what changed since your review on 15 Sep (at c3b4f4d4): - **Tests.** Each of your comments has an answer in its thread: `pattern_has_jit` now `REQUIRE`s a compiled pattern, the extra sentence is gone, the concurrent round is a helper, and the `PCRE2_JIT_TEST_ALLOC` thread is still open for you. The 3 KB case needs no gate, because both engines redirect it. The concurrent test's deep subjects now need the 1 MiB stack, and every round starts at a barrier. - **`RegexMatchContext`.** The constructor assigns the JIT stack callback directly instead of copying the shared context, so it never creates the `thread_local`. Copy assignment frees the old context, and keeps it if the copy can't be allocated. The move operations are real now, since the defaulted ones freed the context twice. The copy and move paths have tests. - **`traffic_layout info`.** Below PCRE2 10.45 it decides `TS_HAS_PCRE2_JIT` by JIT compiling a pattern, so a runtime that can't allocate executable memory reads false. - **regex_remap test.** It moved to Uranium along with master. The checks are the same: the 3 KB query expects the 301, and the 64,000 byte query, on a JIT build only, expects the 200 and the resource error. - **Last two commits.** They only touch comments, to fit the new comment rules, plus one test fix: the self move test now sets a match limit, so it fails if the self move guard goes away. Before, it passed either way. If you want three sittings, I'd take test_Regex.cc first, then Regex.cc and Regex.h, then info.cc and the Uranium test. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
