gnodet-bot commented on code in PR #12694:
URL: https://github.com/apache/maven/pull/12694#discussion_r4046864819
##########
impl/maven-core/src/main/java/org/apache/maven/internal/impl/DefaultLog.java:
##########
@@ -18,21 +18,81 @@
*/
package org.apache.maven.internal.impl;
+import java.lang.StackWalker.StackFrame;
import java.util.function.Supplier;
import org.apache.maven.api.plugin.Log;
+import org.apache.maven.logging.ProjectBuildLogAppender;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
import static java.util.Objects.requireNonNull;
public class DefaultLog implements Log {
+
+ /**
+ * Metadata captured from Log API calls, mirroring the JUL metadata
+ * pattern in {@code MavenJulHandler}.
+ *
+ * @param sourceClassName the fully qualified class name of the caller
+ * @param sourceMethodName the method that issued the log call
+ * @param threadId the originating thread ID
+ */
+ public record LogApiMetadata(String sourceClassName, String
sourceMethodName, long threadId) {}
+
+ private static final ThreadLocal<LogApiMetadata> LOG_API_METADATA = new
ThreadLocal<>();
+ private static final StackWalker WALKER = StackWalker.getInstance();
+ private static final String THIS_CLASS = DefaultLog.class.getName();
+
+ /**
+ * Returns the Log API metadata for the current log event being processed,
+ * or {@code null} if the current event did not originate from the Log API.
+ * <p>
+ * Called from {@code ProjectBuildLogAppender.accept()} to populate
+ * {@code LogEvent.sourceClassName()} and {@code
LogEvent.sourceMethodName()}.
+ *
+ * @return the current Log API metadata, or {@code null}
+ */
+ public static LogApiMetadata getLogApiMetadata() {
+ return LOG_API_METADATA.get();
+ }
+
private final Logger logger;
public DefaultLog(Logger logger) {
this.logger = requireNonNull(logger);
}
+ /**
+ * Wraps a logging call with Log API metadata: sets the ThreadLocal with
+ * source class name and thread ID, executes the actual SLF4J call, and
+ * clears the ThreadLocal.
+ * <p>
+ * The source class name is taken from the SLF4J logger name (which
+ * is the mojo implementation FQCN, set at injection time). The
+ * source method name is resolved via {@link StackWalker} only when
+ * build report capture is active (to avoid the ~1-5μs per-call cost
+ * on every enabled log statement during normal builds).
+ */
+ private void withMetadata(Runnable logAction) {
+ // Only pay the StackWalker cost when someone is actually capturing
metadata
+ String callerMethodName = null;
+ if (ProjectBuildLogAppender.hasReportCapture()) {
+ callerMethodName = WALKER.walk(frames -> frames.dropWhile(f ->
THIS_CLASS.equals(f.getClassName()))
+ .findFirst()
+ .map(StackFrame::getMethodName)
+ .orElse(null));
+ }
+ @SuppressWarnings("deprecation") // Thread.getId() — threadId()
requires Java 19+
+ long threadId = Thread.currentThread().getId();
+ LOG_API_METADATA.set(new LogApiMetadata(logger.getName(),
callerMethodName, threadId));
Review Comment:
**[medium] `LOG_API_METADATA.set()` + `remove()` run unconditionally — not
zero-overhead in normal builds**
The StackWalker call is correctly gated behind `hasReportCapture()`, but
`Thread.currentThread().getId()`, the `new LogApiMetadata(...)` allocation,
`LOG_API_METADATA.set()`, and `LOG_API_METADATA.remove()` execute on every
enabled mojo log call regardless. The PR description says "~1-5μs per-call cost
only when build report capture is active" — that understates the real overhead.
`ThreadLocal.set()` + GC pressure from a `LogApiMetadata` record allocation per
call is non-zero at INFO level across a multi-module build.
Either move the entire `withMetadata` body inside the `hasReportCapture()`
check and fall back to a direct call, or correct the documentation to reflect
the actual cost:
```suggestion
private void withMetadata(Runnable logAction) {
if (ProjectBuildLogAppender.hasReportCapture()) {
// Only pay the StackWalker + ThreadLocal cost when metadata is
needed
String callerMethodName = WALKER.walk(frames ->
frames.dropWhile(f -> THIS_CLASS.equals(f.getClassName()))
.findFirst()
.map(StackFrame::getMethodName)
.orElse(null));
@SuppressWarnings("deprecation") // Thread.getId() — threadId()
requires Java 19+
long threadId = Thread.currentThread().getId();
LOG_API_METADATA.set(new LogApiMetadata(logger.getName(),
callerMethodName, threadId));
try {
logAction.run();
} finally {
LOG_API_METADATA.remove();
}
} else {
logAction.run();
}
}
```
--
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]
