fix(bqjdbc): optimize formatter in BigQueryJdbcRootLogger#12877
fix(bqjdbc): optimize formatter in BigQueryJdbcRootLogger#12877
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the BigQueryJdbcRootLogger to improve logging performance by replacing String.format with StringBuilder and introducing a ThreadLocal for SimpleDateFormat. It also changes how thread names and the process ID are retrieved. The review feedback identifies a potential NumberFormatException when parsing the process ID on certain JVMs, a regression in thread name lookup that fails to account for multiple thread groups, and suggests making the ThreadLocal date formatter static to optimize resource usage.
| Thread[] threads = new Thread[count * 2]; | ||
| int actualCount = rootGroup.enumerate(threads); | ||
| for (int i = 0; i < actualCount; i++) { | ||
| if (threads[i].getId() == threadId) { |
There was a problem hiding this comment.
Maybe worth adding local cache for id->name?
There was a problem hiding this comment.
This fallback is an expensive, low-frequency edge case. The memory overhead of maintaining the cache exceeds the performance gains from faster lookups.
String.formatwithStringBuilderand Guava's Strings for faster padding.Thread.getAllStackTraces()(which triggered heavy JVM safepoints) by usingThread.currentThread()andThread.enumerate()fallbacks.ThreadLocal<SimpleDateFormat>to safely reuse expensive date formatters.