Great work Carter! Have you seen `o.a.l.l.layout.template.json.util.InstantFormatter`, particularly its `Formatter#isInstantMatching` methods used for invalidating the cache? I was thinking of making it even smarter, e.g., if the pattern only has seconds, compare `Instant`s by their seconds. I aspire to pull it to the core, replace access to all individual formatters with this one, and mark the rest deprecated. Another idea I was thinking about is enhancing these individual formatters to contain the precision they require and use that in `isInstantMatching` methods.
Regarding your unicode character problems, shall we try pinging Claes Redestad (@cl4es), who has recently enhanced String.format() in JDK 17 <https://twitter.com/cl4es/status/1432361530268528642>, via Twitter? On Mon, Aug 30, 2021 at 11:32 PM Carter Kozak <[email protected]> wrote: > I've merged the fix for our FixedDateFormat caching bug which caused us to > recompute the same millisecond-precision formatted timestamp unnecessarily > each time our microsecond-precision clock incremented. > https://issues.apache.org/jira/browse/LOG4J2-3153 > > I've also been unwrapping a few layers of complexity, wrapping several > layers of components with conditional logic makes it harder for the jit to > optimize code, so we can improve things by using separate types based on > the requested features: > https://github.com/apache/logging-log4j2/pull/573 > TODO: I'm not happy with the way I unwrapped PatternFormatter objects in > this PR, I think it could work better as an optional wrapper around the > delegate LogEventPatternConverter (where the default FormattingInfo returns > the delegate instance directly) > TODO: simplify MessagePatternConverter a bit, the body is giant for > something that's generally relatively simple. The method is too large for > me to read in a glance, so I imagine the jit will have a hard time making > it fast as well. I don't really like the message-format feature which > allows lookups in the formatted message text because it leaks details of > the framework implementation/configuration into consumers of logging APIs > (which may not even be log4j-core), however I'm not sure how reasonable it > would be to change the default to disallow lookups given I'm sure folks > depend on that behavior. > > I'm not sure what to do about the CharsetEncoder vs > string.getBytes(charset) issue. The CharsetEncoder does not require > allocation and outperforms getBytes when I add a unicode character to the > message. When the message contains only ascii characters, getBytes performs > better. Using CharBuffer.wrap(StringBuilder) produces substantially worse > performance -- it shouldn't be copying the buffer in memory, but I suppose > the heap buffer is more efficient to deal with. I need to do more research > in this area. > > Thoughts/ideas/concerns? > -ck >
