[ 
https://issues.apache.org/jira/browse/GROOVY-12123?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
 ]

Paul King updated GROOVY-12123:
-------------------------------
    Description: 
h2. Summary

Hoist effectively-constant regular expressions to synthetic {{private static 
final Pattern}}
fields so they are compiled once, rather than recompiled on every evaluation -- 
most
importantly inside loops. Transparent optimization; no new user-facing syntax.

h2. Motivation

Groovy's {{~/.../}} operator (and the {{=~}} / {{==~}} operators and JDK 
conveniences such
as {{String#matches}}) construct/compile a {{Pattern}} each time they are 
evaluated. In a
loop, a constant pattern is recompiled every iteration:

{code:groovy}
for (line in lines) {
    if (line ==~ /\d{4}-\d{2}-\d{2}/) { ... } // recompiled every iteration 
today
}
{code}

{{Pattern.compile}} is not cheap, and the pattern here never changes. Lifting 
it to a
compile-once {{static final}} field removes the repeated compilation with no 
change in
observable behaviour.

h2. Proposal

Where the compiler can prove a pattern is *effectively constant*, replace the 
inline
regex expression with a reference to a synthetic {{private static final 
Pattern}} field on
the enclosing class, initialized once. Identical (source + flags) patterns in 
the same
class share a single field.

{code:groovy}
// conceptually, the loop above becomes:
private static final Pattern $pat$0 = Pattern.compile('\\d{4}-\\d{2}-\\d{2}')
...
for (line in lines) {
    if ($pat$0.matcher(line).matches()) { ... }
}
{code}

This is loop-invariant code motion specialized for regex construction. It is 
applied
automatically by the compiler when safe (see _Applicability_ and 
_Correctness_), not via
an annotation.

h2. Applicability ("was applicable")

Only patterns with no runtime-dependent parts qualify:

* {{~/literal/}} with no interpolation -- hoistable.
* {{Pattern.compile("literal"[, constantFlags])}} -- hoistable (method-call 
form).
* {{~"text ${CONST}"}} where the interpolated hole constant-folds -- hoistable 
after folding.
* {{~"text ${var}"}} with a runtime {{var}} -- *not* hoistable; must build each 
time.
* Any pattern closing over a captured, non-constant local -- *not* hoistable.

The primary targets are the {{~/.../}} operator and {{Pattern.compile}}. 
Rewriting the JDK
convenience methods ({{String#matches}}/{{replaceAll}}/{{split}} with a 
constant arg) into a
hoisted-{{Pattern}} form is a larger semantic change (routing through 
{{Pattern}}/{{Matcher}})
and is a possible follow-up extension, not the initial cut.

h2. Correctness / caveats

* *Thread-safety is fine.* {{Pattern}} is immutable and thread-safe, so a shared
  {{static final}} field is safe. {{Matcher}} is stateful and is *never* 
hoisted -- only the
  {{Pattern}}, with a fresh {{Matcher}} per match as today.
* *Exception timing.* An *invalid* constant pattern currently throws 
{{PatternSyntaxException}}
  at the point of evaluation (possibly never, if guarded by a branch). Eager 
static-field
  init would move that to class initialization 
({{ExceptionInInitializerError}}). Mitigation:
  only hoist patterns already known valid (the {{RegexChecker}} type checker 
validates constant
  literals), and/or use a lazy holder so init timing is preserved.
* *Identity.* User code must not rely on each {{~/.../}} yielding a distinct 
{{Pattern}}
  instance ({{===}}); {{Pattern}} carries no meaningful identity or mutability 
contract, so
  sharing is observationally equivalent.

h2. Scope

*In scope*
* Hoisting constant {{~/.../}} and {{Pattern.compile(constant[, constFlags])}} 
to synthetic
  {{static final}} fields, with per-class deduplication of identical patterns.
* Applying inside loops, closures, and script bodies (field on the enclosing 
class).

*Out of scope / deferred*
* Rewriting {{String#matches}}/{{replaceAll}}/{{split}} convenience methods 
(follow-up).
* Hoisting non-constant patterns or any form of runtime {{Pattern}} cache.
* Cross-class / cross-method caching.

h2. Interaction with GROOVY-12122 (Groovy 6)

GROOVY-12122 adds a ReDoS runtime guard ({{RegexGuard}} helper, {{@SafeRegex}} 
scoped
annotation) and lands in Groovy 6. This ticket targets Groovy 7, so it lands 
second and
owns the interaction: by the time it is implemented, the guarded forms below 
are already
shipped and in use.

The two sit on opposite ends of the same operation -- this ticket optimizes 
*Pattern
construction*, GROOVY-12122 guards *matching* -- so they compose, but this pass 
must be
aware of the compile-phase ordering within a single compilation:

* If this pass runs after {{@SafeRegex}} (which rewrites at 
{{SEMANTIC_ANALYSIS}}), it no
  longer sees a bare operator: {{input ==~ /(a+)+$/}} has become
  {{RegexGuard.matchRegex(input, /(a+)+$/, 200)}}. The hoistable pattern is 
then the
  *second* argument of {{matchRegex}} / {{findRegex}}, and the *first* of a 
hand-written
  {{matches}} / {{matcher}}. Note it is whatever the user wrote as the right 
operand -- a
  plain {{String}} constant for {{==~ /literal/}}, or a {{Pattern}} expression 
for
  {{==~ ~/literal/}} -- not necessarily a {{~/.../}} node.
* Hoisting that argument needs no change on the GROOVY-12122 side: the guarded 
entry points
  take the pattern as {{Object}} and use a {{Pattern}} as-is, compiling only
  non-{{Pattern}} values, so substituting a reference to a hoisted {{static 
final Pattern}}
  field simply skips the per-evaluation compile.
* If this pass runs first and produces a {{static final Pattern P}}, nothing 
further is
  needed either: {{@SafeRegex}} rewrites a {{==~}} / {{=~}} expression whatever 
its right
  operand is, so {{input ==~ P}} already guards correctly today.

Desired combined result: the constant {{Pattern}} is compiled once into a 
{{static final}}
field *and* each match wraps the input in the deadline guard.

h2. Implementation notes

* A compiler optimization pass / AST transform over resolved expressions; runs 
in both
  dynamic and {{@CompileStatic}} modes.
* Constant detection reuses the same effectively-constant analysis as constant 
folding.
* Synthetic field naming should be collision-free and deduplicated per class.
* Consider gating behind a flag initially (or restricting to 
{{RegexChecker}}-validated
  patterns) until the exception-timing behaviour is settled.

h2. References

* Related: GROOVY-12122 (ReDoS runtime guard -- RegexGuard / @SafeRegex), 
Groovy 6: https://issues.apache.org/jira/browse/GROOVY-12122
* GROOVY-12122 implementation: https://github.com/apache/groovy/pull/2668
* {{java.util.regex.Pattern}} -- immutable/thread-safe; {{Matcher}} -- not 
thread-safe
* {{RegexChecker}} in groovy-typecheckers (compile-time constant-pattern 
validation)


  was:
h2. Summary

Hoist effectively-constant regular expressions to synthetic {{private static 
final Pattern}}
fields so they are compiled once, rather than recompiled on every evaluation -- 
most
importantly inside loops. Transparent optimization; no new user-facing syntax.

h2. Motivation

Groovy's {{~/.../}} operator (and the {{=~}} / {{==~}} operators and JDK 
conveniences such
as {{String#matches}}) construct/compile a {{Pattern}} each time they are 
evaluated. In a
loop, a constant pattern is recompiled every iteration:

{code:groovy}
for (line in lines) {
    if (line ==~ /\d{4}-\d{2}-\d{2}/) { ... } // recompiled every iteration 
today
}
{code}

{{Pattern.compile}} is not cheap, and the pattern here never changes. Lifting 
it to a
compile-once {{static final}} field removes the repeated compilation with no 
change in
observable behaviour.

h2. Proposal

Where the compiler can prove a pattern is *effectively constant*, replace the 
inline
regex expression with a reference to a synthetic {{private static final 
Pattern}} field on
the enclosing class, initialized once. Identical (source + flags) patterns in 
the same
class share a single field.

{code:groovy}
// conceptually, the loop above becomes:
private static final Pattern $pat$0 = Pattern.compile('\\d{4}-\\d{2}-\\d{2}')
...
for (line in lines) {
    if ($pat$0.matcher(line).matches()) { ... }
}
{code}

This is loop-invariant code motion specialized for regex construction. It is 
applied
automatically by the compiler when safe (see _Applicability_ and 
_Correctness_), not via
an annotation.

h2. Applicability ("was applicable")

Only patterns with no runtime-dependent parts qualify:

* {{~/literal/}} with no interpolation -- hoistable.
* {{Pattern.compile("literal"[, constantFlags])}} -- hoistable (method-call 
form).
* {{~"text ${CONST}"}} where the interpolated hole constant-folds -- hoistable 
after folding.
* {{~"text ${var}"}} with a runtime {{var}} -- *not* hoistable; must build each 
time.
* Any pattern closing over a captured, non-constant local -- *not* hoistable.

The primary targets are the {{~/.../}} operator and {{Pattern.compile}}. 
Rewriting the JDK
convenience methods ({{String#matches}}/{{replaceAll}}/{{split}} with a 
constant arg) into a
hoisted-{{Pattern}} form is a larger semantic change (routing through 
{{Pattern}}/{{Matcher}})
and is a possible follow-up extension, not the initial cut.

h2. Correctness / caveats

* *Thread-safety is fine.* {{Pattern}} is immutable and thread-safe, so a shared
  {{static final}} field is safe. {{Matcher}} is stateful and is *never* 
hoisted -- only the
  {{Pattern}}, with a fresh {{Matcher}} per match as today.
* *Exception timing.* An *invalid* constant pattern currently throws 
{{PatternSyntaxException}}
  at the point of evaluation (possibly never, if guarded by a branch). Eager 
static-field
  init would move that to class initialization 
({{ExceptionInInitializerError}}). Mitigation:
  only hoist patterns already known valid (the {{RegexChecker}} type checker 
validates constant
  literals), and/or use a lazy holder so init timing is preserved.
* *Identity.* User code must not rely on each {{~/.../}} yielding a distinct 
{{Pattern}}
  instance ({{===}}); {{Pattern}} carries no meaningful identity or mutability 
contract, so
  sharing is observationally equivalent.

h2. Scope

*In scope*
* Hoisting constant {{~/.../}} and {{Pattern.compile(constant[, constFlags])}} 
to synthetic
  {{static final}} fields, with per-class deduplication of identical patterns.
* Applying inside loops, closures, and script bodies (field on the enclosing 
class).

*Out of scope / deferred*
* Rewriting {{String#matches}}/{{replaceAll}}/{{split}} convenience methods 
(follow-up).
* Hoisting non-constant patterns or any form of runtime {{Pattern}} cache.
* Cross-class / cross-method caching.

h2. Relates to / must compose with GROOVY-12122

GROOVY-12122 adds a ReDoS runtime guard ({{RegexGuard}} helper, {{@SafeRegex}} 
scoped
annotation). The two touch the same regex AST surface and sit on opposite ends 
of the same
operation -- this ticket optimizes *Pattern construction*, GROOVY-12122 guards 
*matching* --
so they compose, but the transforms must be ordering-aware:

* When {{@SafeRegex}} rewrites {{input ==~ /(a+)+$/}} to
  {{RegexGuard.matches(~/(a+)+$/, input, ...)}}, this optimization must still 
recognize the
  {{~/(a+)+$/}} argument as a hoistable constant and lift it.
* When this optimization runs first and produces a {{static final Pattern P}}, 
{{@SafeRegex}}
  must still recognize a match against {{P}} and wrap the input.

Desired combined result: the constant {{Pattern}} is compiled once into a 
{{static final}}
field *and* each match wraps the input in the deadline guard. Whoever 
implements the second
of the two should account for the first.

h2. Implementation notes

* A compiler optimization pass / AST transform over resolved expressions; runs 
in both
  dynamic and {{@CompileStatic}} modes.
* Constant detection reuses the same effectively-constant analysis as constant 
folding.
* Synthetic field naming should be collision-free and deduplicated per class.
* Consider gating behind a flag initially (or restricting to 
{{RegexChecker}}-validated
  patterns) until the exception-timing behaviour is settled.

h2. References

* Related: GROOVY-12122 (ReDoS runtime guard -- RegexGuard / @SafeRegex): 
https://issues.apache.org/jira/browse/GROOVY-12122
* {{java.util.regex.Pattern}} -- immutable/thread-safe; {{Matcher}} -- not 
thread-safe
* {{RegexChecker}} in groovy-typecheckers (compile-time constant-pattern 
validation)


> Hoist constant regexes to static final fields
> ---------------------------------------------
>
>                 Key: GROOVY-12123
>                 URL: https://issues.apache.org/jira/browse/GROOVY-12123
>             Project: Groovy
>          Issue Type: Improvement
>            Reporter: Paul King
>            Assignee: Paul King
>            Priority: Major
>             Fix For: 7.x
>
>
> h2. Summary
> Hoist effectively-constant regular expressions to synthetic {{private static 
> final Pattern}}
> fields so they are compiled once, rather than recompiled on every evaluation 
> -- most
> importantly inside loops. Transparent optimization; no new user-facing syntax.
> h2. Motivation
> Groovy's {{~/.../}} operator (and the {{=~}} / {{==~}} operators and JDK 
> conveniences such
> as {{String#matches}}) construct/compile a {{Pattern}} each time they are 
> evaluated. In a
> loop, a constant pattern is recompiled every iteration:
> {code:groovy}
> for (line in lines) {
>     if (line ==~ /\d{4}-\d{2}-\d{2}/) { ... } // recompiled every iteration 
> today
> }
> {code}
> {{Pattern.compile}} is not cheap, and the pattern here never changes. Lifting 
> it to a
> compile-once {{static final}} field removes the repeated compilation with no 
> change in
> observable behaviour.
> h2. Proposal
> Where the compiler can prove a pattern is *effectively constant*, replace the 
> inline
> regex expression with a reference to a synthetic {{private static final 
> Pattern}} field on
> the enclosing class, initialized once. Identical (source + flags) patterns in 
> the same
> class share a single field.
> {code:groovy}
> // conceptually, the loop above becomes:
> private static final Pattern $pat$0 = Pattern.compile('\\d{4}-\\d{2}-\\d{2}')
> ...
> for (line in lines) {
>     if ($pat$0.matcher(line).matches()) { ... }
> }
> {code}
> This is loop-invariant code motion specialized for regex construction. It is 
> applied
> automatically by the compiler when safe (see _Applicability_ and 
> _Correctness_), not via
> an annotation.
> h2. Applicability ("was applicable")
> Only patterns with no runtime-dependent parts qualify:
> * {{~/literal/}} with no interpolation -- hoistable.
> * {{Pattern.compile("literal"[, constantFlags])}} -- hoistable (method-call 
> form).
> * {{~"text ${CONST}"}} where the interpolated hole constant-folds -- 
> hoistable after folding.
> * {{~"text ${var}"}} with a runtime {{var}} -- *not* hoistable; must build 
> each time.
> * Any pattern closing over a captured, non-constant local -- *not* hoistable.
> The primary targets are the {{~/.../}} operator and {{Pattern.compile}}. 
> Rewriting the JDK
> convenience methods ({{String#matches}}/{{replaceAll}}/{{split}} with a 
> constant arg) into a
> hoisted-{{Pattern}} form is a larger semantic change (routing through 
> {{Pattern}}/{{Matcher}})
> and is a possible follow-up extension, not the initial cut.
> h2. Correctness / caveats
> * *Thread-safety is fine.* {{Pattern}} is immutable and thread-safe, so a 
> shared
>   {{static final}} field is safe. {{Matcher}} is stateful and is *never* 
> hoisted -- only the
>   {{Pattern}}, with a fresh {{Matcher}} per match as today.
> * *Exception timing.* An *invalid* constant pattern currently throws 
> {{PatternSyntaxException}}
>   at the point of evaluation (possibly never, if guarded by a branch). Eager 
> static-field
>   init would move that to class initialization 
> ({{ExceptionInInitializerError}}). Mitigation:
>   only hoist patterns already known valid (the {{RegexChecker}} type checker 
> validates constant
>   literals), and/or use a lazy holder so init timing is preserved.
> * *Identity.* User code must not rely on each {{~/.../}} yielding a distinct 
> {{Pattern}}
>   instance ({{===}}); {{Pattern}} carries no meaningful identity or 
> mutability contract, so
>   sharing is observationally equivalent.
> h2. Scope
> *In scope*
> * Hoisting constant {{~/.../}} and {{Pattern.compile(constant[, 
> constFlags])}} to synthetic
>   {{static final}} fields, with per-class deduplication of identical patterns.
> * Applying inside loops, closures, and script bodies (field on the enclosing 
> class).
> *Out of scope / deferred*
> * Rewriting {{String#matches}}/{{replaceAll}}/{{split}} convenience methods 
> (follow-up).
> * Hoisting non-constant patterns or any form of runtime {{Pattern}} cache.
> * Cross-class / cross-method caching.
> h2. Interaction with GROOVY-12122 (Groovy 6)
> GROOVY-12122 adds a ReDoS runtime guard ({{RegexGuard}} helper, 
> {{@SafeRegex}} scoped
> annotation) and lands in Groovy 6. This ticket targets Groovy 7, so it lands 
> second and
> owns the interaction: by the time it is implemented, the guarded forms below 
> are already
> shipped and in use.
> The two sit on opposite ends of the same operation -- this ticket optimizes 
> *Pattern
> construction*, GROOVY-12122 guards *matching* -- so they compose, but this 
> pass must be
> aware of the compile-phase ordering within a single compilation:
> * If this pass runs after {{@SafeRegex}} (which rewrites at 
> {{SEMANTIC_ANALYSIS}}), it no
>   longer sees a bare operator: {{input ==~ /(a+)+$/}} has become
>   {{RegexGuard.matchRegex(input, /(a+)+$/, 200)}}. The hoistable pattern is 
> then the
>   *second* argument of {{matchRegex}} / {{findRegex}}, and the *first* of a 
> hand-written
>   {{matches}} / {{matcher}}. Note it is whatever the user wrote as the right 
> operand -- a
>   plain {{String}} constant for {{==~ /literal/}}, or a {{Pattern}} 
> expression for
>   {{==~ ~/literal/}} -- not necessarily a {{~/.../}} node.
> * Hoisting that argument needs no change on the GROOVY-12122 side: the 
> guarded entry points
>   take the pattern as {{Object}} and use a {{Pattern}} as-is, compiling only
>   non-{{Pattern}} values, so substituting a reference to a hoisted {{static 
> final Pattern}}
>   field simply skips the per-evaluation compile.
> * If this pass runs first and produces a {{static final Pattern P}}, nothing 
> further is
>   needed either: {{@SafeRegex}} rewrites a {{==~}} / {{=~}} expression 
> whatever its right
>   operand is, so {{input ==~ P}} already guards correctly today.
> Desired combined result: the constant {{Pattern}} is compiled once into a 
> {{static final}}
> field *and* each match wraps the input in the deadline guard.
> h2. Implementation notes
> * A compiler optimization pass / AST transform over resolved expressions; 
> runs in both
>   dynamic and {{@CompileStatic}} modes.
> * Constant detection reuses the same effectively-constant analysis as 
> constant folding.
> * Synthetic field naming should be collision-free and deduplicated per class.
> * Consider gating behind a flag initially (or restricting to 
> {{RegexChecker}}-validated
>   patterns) until the exception-timing behaviour is settled.
> h2. References
> * Related: GROOVY-12122 (ReDoS runtime guard -- RegexGuard / @SafeRegex), 
> Groovy 6: https://issues.apache.org/jira/browse/GROOVY-12122
> * GROOVY-12122 implementation: https://github.com/apache/groovy/pull/2668
> * {{java.util.regex.Pattern}} -- immutable/thread-safe; {{Matcher}} -- not 
> thread-safe
> * {{RegexChecker}} in groovy-typecheckers (compile-time constant-pattern 
> validation)



--
This message was sent by Atlassian Jira
(v8.20.10#820010)

Reply via email to