Skip to content

fix(bqjdbc): optimize formatter in BigQueryJdbcRootLogger#12877

Merged
Neenu1995 merged 5 commits intomainfrom
ns/optimize-log-formatter
Apr 22, 2026
Merged

fix(bqjdbc): optimize formatter in BigQueryJdbcRootLogger#12877
Neenu1995 merged 5 commits intomainfrom
ns/optimize-log-formatter

Conversation

@Neenu1995
Copy link
Copy Markdown
Contributor

@Neenu1995 Neenu1995 commented Apr 21, 2026

  • Replaced String.format with StringBuilder and Guava's Strings for faster padding.
  • Eliminated Thread.getAllStackTraces() (which triggered heavy JVM safepoints) by using Thread.currentThread() and Thread.enumerate() fallbacks.
  • Cached PROCESS_ID instead of parsing it dynamically on every log line.
  • Swapped in ThreadLocal<SimpleDateFormat> to safely reuse expensive date formatters.

@Neenu1995 Neenu1995 requested review from a team as code owners April 21, 2026 17:23
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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) {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe worth adding local cache for id->name?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This fallback is an expensive, low-frequency edge case. The memory overhead of maintaining the cache exceeds the performance gains from faster lookups.

@Neenu1995 Neenu1995 merged commit 4233faf into main Apr 22, 2026
112 of 114 checks passed
@Neenu1995 Neenu1995 deleted the ns/optimize-log-formatter branch April 22, 2026 16:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants