Memoize system environment resolution in NavigableMapConfig

[codeconsole]

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:

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:

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:

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.

On the @Deprecated marker

NavigableMapConfig carries a class-level @Deprecated since #11554 (May 2020), pointing at grails.config.Config. That target is the interface this class implements, and no alternative implementation exists in the tree, so the note reads as "code against the interface" rather than "a replacement is available".

The class is on the live path today:

• PropertySourcesConfig extends NavigableMapConfig and is not itself deprecated.
• AbstractGrailsApplication builds a PropertySourcesConfig, so this is grailsApplication.config for every running application.
• Profiling a running 8.0.0-M5 application put NavigableMapConfig at 7.01% of execution samples, entered through getProperty.

If the NavigableMap-backed implementation is replaced, this change goes with it. It is offered as a cost reduction on the path applications actually execute for the six years the marker has been in place, and is easy to drop if a replacement is in flight.

[codecov]

Codecov Report

❌ Patch coverage is 68.42105% with 6 lines in your changes missing coverage. Please review.
✅ Project coverage is 52.4622%. Comparing base (b9f5c1f) to head (5ed8094).
⚠️ Report is 1582 commits behind head on 8.0.x.

Files with missing lines	Patch %	Lines
...n/groovy/org/grails/config/NavigableMapConfig.java	68.4210%	1 Missing and 5 partials ⚠️

Additional details and impacted files

@@                Coverage Diff                 @@
##                8.0.x     #16130        +/-   ##
==================================================
+ Coverage     52.3676%   52.4622%   +0.0947%     
- Complexity      18313      18322         +9     
==================================================
  Files            2036       2032         -4     
  Lines           96365      96315        -50     
  Branches        16832      16840         +8     
==================================================
+ Hits            50464      50529        +65     
+ Misses          38480      38354       -126     
- Partials         7421       7432        +11     

Files with missing lines	Coverage Δ
...n/groovy/org/grails/config/NavigableMapConfig.java	68.0233% <68.4210%> (+0.3017%)	⬆️

... and 12 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[testlens-app]

🔎 No tests executed 🔎

🏷️ Commit: 5ed8094
▶️ Tests: 0 executed
⚪️ Checks: 79/79 completed

---

Learn more about TestLens at testlens.app/docs.

[sbglasius]

Suggested change

	assert config.getProperty('some.nested.value') == 'original'
	expect:
	config.getProperty('some.nested.value') == 'original'

[sbglasius]

Suggested change

	assert before.getProperty('late.bound.property') == 'from-yml'
	expect:
	before.getProperty('late.bound.property') == 'from-yml'

[codeconsole]

@jdaugherty suggests doing merge at the start and remove deprecated.

[jdaugherty]

@codeconsole I think we need to look at the histoyro f the changes on the navigablemap. from our call yesterday:

2016 – #10188: buggy/awkward nested-map replacement semantics are identified; Jeff investigates and adds a failing test.
2016 – #10280: Jeff starts an experimental implementation to fix the semantics. It never gets merged.
2020 – #11554: NavigableMap is deprecated, making Jeff's unfinished approach ostensibly unnecessary.
2024 – #13869: Grails reverses course enough to restore dot-notation navigation for Grails 7.
2025 – #14005: the project explicitly resurrects #10188/#10280 as the underlying problems that should now actually be fixed.

I don't think memoized is the solution here b/c it's a cache and with large configuration you're going to be using a lot of memory. Can't we look at what Jeff did originally and see if we can fix it that way?

[jdaugherty]

I've moved this to 8.1 for now. It will likely land in 9 though.