logging-log4j2
logging-log4j2 copied to clipboard
ThrowableProxy pre-warms its cache using the detected stack
This allows us to avoid class loading even when order doesn't match the stack due to exceptions caught and pased from a slightly different codepath. This change also includes a name-based predicate to avoid loading classes types which are known not to provide location information. Unfortunately this mostly invalidates our existing benchmarks which are based on dynamic proxies, but modern java applications use proxies and lambdas heavily enough that I expect a sizable performance improvement.
In a future commit we may consider relying exclusively on classes from the detected stack for location information and entirely avoid class loading.
I need to think about this longer before I'm comfortable with it. Posted for early feedback on the idea.
@carterkozak Have you revisited this? To be honest I looked at the review here but it is hard to tell what effect it might have. It would be great to have some JMH benchmarks to indicate what kind of improvement it provides. Also, I'd like to know if there will be any side-effects or consequences of the change.
I had forgotten about this change. I'm a little bit afraid of changes to ThrowableProxy because there are a number of cases when multiple classes with the same fully qualified name come from separate loaders in a single trace. I'm not sure this PR would necessarily regress those cases, but I'm sure we could come up with scenarios where each implementation is more accurate. I'll revisit this when I have some more time, it has been a crazy week.
Looking at this again, I think I should extract the predicate for classes which can't be loaded to a separate change. It should be generally helpful because the cost of a failed class load is very high. While I don't think it solves the problem, it may reduce the magnitude. Separately I can document the pros and cons of this approach in which we avoid re-loading classes with names that appear in the collected stack.
Looking at this again, I think I should extract the predicate for classes which can't be loaded to a separate change. It should be generally helpful because the cost of a failed class load is very high. While I don't think it solves the problem, it may reduce the magnitude.
+1; I think that fix would be very helpful.
Apache Spark is upgrading to Log4J 2 in its 3.3 release and during testing I spotted performance problems due to unloadable classes in error stacktraces. Because of this performance issue, I am proposing to change Spark
s default Log4J configuration to specify an explicit %ex formatter so we don't use the extended exception information: https://github.com/apache/spark/pull/36747