jamesfredley opened a new pull request, #15800:
URL: https://github.com/apache/grails-core/pull/15800
# Reduce cached codec metaclass registration overhead
## Summary
This pull request reduces repeated `encodeAs*` and `decode*`
ExpandoMetaClass writes during cached codec registration, adds focused
compatibility coverage, and includes repeatable benchmark support plus real
Grails application validation. Re-registering the same cached codec factory for
the same target class and codec method now avoids mutating the target metaclass
after the first successful registration.
The public Grails codec surface remains unchanged. Existing applications
continue to declare codecs and call methods such as `value.encodeAsHTML()`,
`value.encodeAsURL()`, and `value.decodeSomeCodec()` in the same way as before.
Fixes apache/grails-core#15374.
## Problem
Grails codec registration dynamically adds `encodeAs*` and `decode*` methods
to globally hot metaclasses:
- `String`
- `GStringImpl`
- `StringBuffer`
- `StringBuilder`
- `Object`
On Groovy Indy, repeated ExpandoMetaClass mutation of these globally used
types can invalidate call sites and create avoidable startup/runtime overhead.
The most important compatible win is to stop adding the exact same cached codec
method repeatedly when the same `CodecFactory` is already registered for the
same target metaclass.
## What changed
This is the codec-registration slice of the broader #15374 / Tier 1 / Tier 4
investigation. The research also examined GORM dynamic methods, taglib
dispatch, static compilation opportunities, artefact indexing, and metaclass
freeze or warning mechanisms, but those areas are not changed here because they
carry a larger compatibility and review surface. This PR keeps the shipped code
focused on the automatic, backward-compatible codec metaclass registration win
that produced measurable before/after gains.
### Runtime behavior
- `CodecMetaClassSupport` now tracks cached metaclass registrations by
target class, codec method name, and `CodecFactory` identity.
- `CodecFactory` identity is represented by a weak identity key, not by
codec name and not by raw `identityHashCode` alone. This avoids collapsing
distinct factories that happen to expose the same codec name.
- Repeated cached registration for the same factory and method skips the
second and later ExpandoMetaClass write.
- The first registration still adds the normal `encodeAs*` / `decode*`
method closures.
- If a target metaclass was removed and recreated, registration re-adds the
method even when the same factory identity was seen before.
- Cached duplicate checks are synchronized per target `ExpandoMetaClass` so
parallel registration cannot race into duplicate writes for the same metaclass.
- Internal test/benchmark support can clear the registration state. This is
not a new user-facing lifecycle API.
### Compatibility-preserving cases
- Non-cached registration is unchanged. This preserves development/reload
behavior where codecs may need to be resolved and reattached repeatedly.
- Distinct `CodecFactory` instances are still treated as distinct even if
they expose the same codec name. This avoids collapsing factories that might
intentionally provide different encoder/decoder instances behind the same codec
identifier.
- Existing aliases continue to be registered through the same path as
primary codec names.
- No application code changes are required.
### Verification support
- Added `CodecMetaClassSupportSpec` coverage for:
- cached same-factory registration idempotence
- cached registration after metaclass replacement
- distinct cached factories remaining isolated
- non-cached registration continuing to re-register
- Added a `CodecMetaClassBenchmark` test main plus
`:grails-encoder:codecMetaClassBenchmark` Gradle task to make the before/after
registration cost observable.
- The benchmark task forwards `grails.codec.benchmark.*` system properties
to its JavaExec fork so registration counts and modes can be adjusted from the
command line.
## Backward compatibility for Grails applications
This change is intended to be fully backward compatible for Grails
applications upgrading to a release that includes it.
### Required app changes
None.
Applications do not need to change codec declarations, GSPs, taglibs,
controllers, services, or calls to `encodeAs*` / `decode*`.
### Public API compatibility
- The user-facing dynamic methods are unchanged.
- The codec lookup and registration entry points are unchanged.
- No dependency changes were made.
- No configuration property changes were made.
- No migration step is required.
### Runtime compatibility
- Cached production-style codec lookup still resolves the encoder/decoder
once and installs the same closures as before.
- Development/non-cached codec lookup still resolves codecs at invocation
time and still re-registers. This preserves reload-oriented behavior.
- Distinct codec factory instances are not deduplicated against each other.
This is intentionally conservative for plugins or applications that build
factories dynamically.
- If a metaclass is replaced, the method is re-added on the new metaclass,
which avoids stale static bookkeeping preventing a fresh metaclass from
receiving the codec method.
### Behavioral compatibility
The expected rendered/encoded values are unchanged. Functional GSP coverage
verifies that HTML and URL encoding still works through the browser-facing
application surface.
## Documentation
No user-facing documentation change is required because this PR does not add
a new feature, configuration option, public API, or migration step. The
existing codec usage documentation remains accurate because application code
still uses the same `encodeAs*` and `decode*` methods.
Documentation-related notes for reviewers:
- There is no new setting to document.
- There is no new user workflow to document.
- There is no application upgrade action to document.
- There is no changed codec syntax to document.
- The benchmark task is test/developer verification infrastructure, not an
end-user feature.
If the project wants a release note, the suggested wording is:
> Improve codec metaclass registration performance by avoiding repeated
cached `encodeAs*` / `decode*` ExpandoMetaClass writes while preserving
existing codec behavior and development reload semantics.
## Testing and verification
### Unit and module tests
Passed:
```text
./gradlew :grails-encoder:clean :grails-encoder:test -PmaxTestParallel=1
```
Also rerun after the final hardening pass:
```text
./gradlew :grails-encoder:test -PmaxTestParallel=1
```
The encoder module test run includes the new `CodecMetaClassSupportSpec`
coverage plus existing encoder, HTML encoder, JavaScript codec, chained
encoder, and `StreamCharBuffer` coverage.
### Functional surface test
Passed:
```text
./gradlew :grails-test-examples-gsp-layout:integrationTest \
--tests "GspTagLibSpec.encodeAsHTML prevents XSS" \
--tests "GspTagLibSpec.encodeAsURL encodes URL parameters" \
-PmaxTestParallel=1 \
--rerun-tasks
```
This verifies the change through a real Grails application surface using GSP
rendering and browser-driven Geb/Testcontainers execution.
Note: an earlier GSP attempt failed while starting Selenium/Testcontainers
and left orphaned `selenium/standalone-chrome:4.43.0` containers. After
removing those orphaned containers, the targeted GSP codec specs passed.
### Benchmarks
Benchmark command shape:
```text
./gradlew -q \
"-Dgrails.codec.benchmark.registrationIterations=10000" \
"-Dgrails.codec.benchmark.encodeWarmupIterations=100000" \
"-Dgrails.codec.benchmark.encodeIterations=1000000" \
:grails-encoder:codecMetaClassBenchmark
```
Distinct-factory control adds:
```text
"-Dgrails.codec.benchmark.newFactoryEachRegistration=true"
```
Observed local results:
| Scenario | Branch | registrationNanos | registrationNanosPerOp |
codecMetaMethodRegistrations |
| --- | --- | ---: | ---: | ---: |
| same factory, 10,000 registrations | baseline `origin/8.0.x` with copied
harness | 7,988,652,900 | 798,865.29 | counter unavailable |
| same factory, 10,000 registrations | this branch | 1,233,321,800 |
123,332.18 | 2 |
| new factory per registration, 10,000 registrations | baseline
`origin/8.0.x` with copied harness | 9,961,345,100 | 996,134.51 | counter
unavailable |
| new factory per registration, 10,000 registrations | this branch |
14,439,459,900 | 1,443,945.99 | 20,000 |
The compatible same-factory cached path improved from about `7.99s` to about
`1.23s` for 10,000 registrations in this local harness, roughly a 6.5x
reduction in registration time. The final branch also confirms only two
metaclass method writes for the single encoder-only benchmark codec in
same-factory mode.
The new-factory control still performs 20,000 method writes for 10,000
registrations because each iteration uses a distinct factory and the PR
intentionally preserves that compatibility behavior. That control is not the
optimized compatibility path; it exists to prove distinct factories are not
collapsed.
### Real Grails application benchmark
To verify the improvement was visible outside the micro-harness, I added and
ran an opt-in real-app benchmark against `grails-test-examples-app1`. This is a
full Grails application using the web plugin, GSP plugin, Hibernate 5, cache,
scaffolding, Sitemesh/layout support, local test plugins, and the existing
`CodecTestController` integration surface.
The benchmark starts the actual app with `server.port=0`, then exercises:
- full application startup and codec registration
- direct dynamic codec calls inside the initialized app
- HTTP requests against all existing codec controller endpoints
The same benchmark harness was copied into an `origin/8.0.x` baseline
worktree for apples-to-apples comparison.
Combined workload command shape:
```text
./gradlew -q \
"-Dgrails.codec.realapp.startupIterations=5" \
"-Dgrails.codec.realapp.httpRounds=10" \
"-Dgrails.codec.realapp.directRounds=20000" \
:grails-test-examples-app1:realAppCodecBenchmark
```
This workload performs 5 full app starts, 900 total HTTP codec endpoint
requests, and 800,000 total direct dynamic codec operations.
| Metric | Baseline `origin/8.0.x` | This branch | Improvement |
| --- | ---: | ---: | ---: |
| Startup mean, 5 app starts | `16.36s` | `11.26s` | 31.2% lower, 1.45x
faster |
| Startup median, 5 app starts | `10.03s` | `5.86s` | 41.6% lower, 1.71x
faster |
| Direct dynamic codec median per startup, 160,000 ops/startup | `8.11s` |
`6.48s` | 20.1% lower, 1.25x faster |
| HTTP codec endpoint median per startup, 180 requests/startup | `2.10s` |
`1.53s` | 26.8% lower, 1.37x faster |
Startup-only command shape:
```text
./gradlew -q \
"-Dgrails.codec.realapp.startupIterations=8" \
"-Dgrails.codec.realapp.httpRounds=0" \
"-Dgrails.codec.realapp.directRounds=0" \
:grails-test-examples-app1:realAppCodecBenchmark
```
| Metric | Baseline `origin/8.0.x` | This branch | Improvement |
| --- | ---: | ---: | ---: |
| Startup mean, 8 app starts | `16.94s` | `10.48s` | 38.1% lower, 1.62x
faster |
| Startup median, 8 app starts | `9.55s` | `5.93s` | 37.9% lower, 1.61x
faster |
Branch-side registration counter evidence from the real app:
- 5-start combined workload: `realAppMetaMethodRegistrationsDelta=550`
- 8-start startup-only workload: `realAppMetaMethodRegistrationsDelta=880`
That is exactly 110 codec metaclass writes per full `app1` startup on this
branch. The baseline does not expose the counter, so the direct before/after
comparison uses startup and workload timings.
### Static/diff checks
Passed:
```text
git diff --check
```
LSP diagnostics were run on the changed Groovy/Gradle files. The Gradle file
was clean. The Groovy diagnostics still report unresolved classpath warnings
for standard Groovy/Grails/Spock classes in this workspace, while Gradle
compilation and tests pass.
## Risk assessment
### Low-risk aspects
- No user-facing API changed.
- No dependency changed.
- No application configuration changed.
- The optimization is limited to cached lookup mode.
- Existing non-cached behavior is preserved.
- Distinct factory instances remain distinct.
### Main risks considered
#### Metaclass replacement
Risk: static bookkeeping could incorrectly skip registration after a target
metaclass is removed and recreated.
Mitigation: the registration check also verifies that the target metaclass
currently has the method. A dedicated test removes the target metaclass and
confirms the same cached factory re-adds the method.
#### Parallel registration
Risk: concurrent cached registration could race and both threads could write
the same method.
Mitigation: duplicate checks and writes are synchronized per target
`ExpandoMetaClass`.
#### Dynamic factory behavior
Risk: two different factories with the same codec name could intentionally
behave differently.
Mitigation: the registration key includes weak `CodecFactory` identity, so
distinct live factories are still registered separately without retaining
factory instances solely for bookkeeping.
#### Static registry lifecycle
Risk: a JVM-wide registration registry can outlive individual tests or
application contexts.
Mitigation: tests clear the internal registration state and remove touched
metaclasses. The registry uses weak factory references so bookkeeping does not
keep codec factories alive. Stale key objects can remain until the internal
state is cleared, but they do not retain the factory and do not collapse a
later distinct live factory.
#### Development reload behavior
Risk: deduplication could prevent changed codecs from being reattached in
development-style non-cached lookup.
Mitigation: deduplication is not applied when `cacheLookup` is false. The
new tests verify re-registration in non-cached mode.
## Reviewer notes
- The benchmark counter is intentionally unavailable on baseline because the
counter is part of this PR. Baseline timing is still comparable because the
same benchmark harness was copied into a baseline worktree without the
production optimization.
- `metaClassChangeEvents` remained zero in the benchmark because Groovy's
registry listener did not observe these ExpandoMetaClass method additions in
the local harness. The PR therefore uses an internal registration counter for
branch-side write-count evidence.
- The optimization targets repeated registration overhead. It does not
attempt to redesign Grails codec dispatch, remove dynamic codec methods, or
change how codecs are discovered.
--
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]
