codeconsole commented on PR #16130: URL: https://github.com/apache/grails-core/pull/16130#issuecomment-6329819615
@jdaugherty I went through the five items. Jeff's work on #10188 and #10280 is a `replace(Map)` for the merge-versus-replace semantics of nested maps in `NavigableMap`, which is what the `navigableMapConfigReplace/issue10188` branch still holds; it never touched the lookup path, and there is no faster resolution design in that thread. The cost here comes from a 2018 change that probes the process environment first on every `getProperty`, with up to eight `System.getenv` spellings copied from Spring's `SystemEnvironmentPropertySource`, before the configuration is even consulted. On memory: the memoization was keyed by the looked-up key and capped, so it did not grow with the configuration, but I agree a per-key cache is the wrong shape for this. The PR now does what Spring Boot's binder does with the system environment: snapshot it once per config and index the variable names by canonical form, so a key no variable can match is rejected with one hash lookup, and a key that can match runs the unchanged spelling-by-spelling resolution against the snapshot. Memory is proportional to the number of environment variables, nothing is cached per key, and nothing is evicted. `NavigableMap.get` is also compiled statically now; it carried `@CompileDynamic` over a one-line body, which put dynamic dispatch on every segment of a dotted walk. Numbers and the behaviour notes are in the description: 370 → 152 ns per lookup here, with the full grails-core and grails-bootstrap suites green. The replace semantics of #10188 are untouched and stay a separate change. -- 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]
