codeconsole opened a new pull request, #16130:
URL: https://github.com/apache/grails-core/pull/16130
## What
`Config.getProperty` resolves every key against the process environment from
scratch on each call.
`findInSystemEnvironment` asks `resolvePropertyName` for the environment
spelling of the key, and `checkPropertyName` probes `System.getenv` up to four
times per casing — as-is, dots replaced with underscores, hyphens replaced with
underscores, both replaced — then repeats the whole sequence against the
uppercased key:
```java
private String checkPropertyName(String name) {
if (containsKey(name)) return name; // containsKey
== System.getenv(name) != null
String noDotName = name.replace('.', '_');
if (!name.equals(noDotName) && containsKey(noDotName)) return noDotName;
String noHyphenName = name.replace('-', '_');
...
}
```
That is up to eight `getenv` probes and six intermediate strings per lookup,
and the answer cannot change — the process environment is fixed for the
lifetime of a config.
This memoizes the resolution per config instance, along with the token list
a dotted key splits into:
```groovy
config.getProperty('grails.views.gsp.encoding', String) // resolves the
environment once
config.getProperty('grails.views.gsp.encoding', String) // subsequent
calls reuse it
```
Property **values** are not cached, so configuration mutated at runtime is
still observed:
```groovy
config.getProperty('some.nested.value') // 'original'
config.merge(['some.nested.value': 'updated'])
config.getProperty('some.nested.value') // 'updated'
```
The cache is per-instance rather than static on purpose.
`SystemEnvironmentConfigSpec` installs environment variables reflectively and
then builds a fresh config, which must observe the environment as it stands at
that point; a static cache would leak a stale answer across those specs.
## Why it matters
Callers resolve configuration inside render loops. On a scaffolded page
rendering 100 rows, `FormFieldsTemplateService` (`getShouldCache`,
`findTemplate`, `getTemplateFor`) and asset-pipeline resolve configuration per
rendered property, so this sits on a hot path that scales with rows ×
properties.
## Measurements
| | before | after |
|---|---|---|
| `getProperty`, 2M lookups over a 4-key set | 355.3 ns/op | 91.6 ns/op |
| `NavigableMapConfig` share of JFR execution samples, 100-row scaffolded
page under load | 7.01% | 1.16% |
## Limitations
- **End-to-end request latency improvement was not demonstrated.** The CPU
work is measurably removed (both figures above), but on the machine used for
this the run-to-run variance across JVM restarts was larger than the effect, so
wall-clock A/B could not resolve it. The dominant remaining costs on that page
are `ExpandoMetaClass` read-lock contention (~9–10% of samples) and reflective
tag dispatch, neither of which this touches.
- The caches are bounded at 2048 entries each. Configuration keys come from
a small fixed set of source literals; the bound only guards against a caller
synthesising unbounded key strings at runtime.
- `NavigableMapConfig` is deprecated. This is a cost reduction on the
existing path, not an endorsement of it.
--
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]
