This is an automated email from the ASF dual-hosted git repository.
gengliang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new 9141aa4bb2af [SPARK-48744][CORE] Log entry should be constructed only
once
9141aa4bb2af is described below
commit 9141aa4bb2af7ce476a81c2934511b75b17cde90
Author: Gengliang Wang <[email protected]>
AuthorDate: Thu Jun 27 21:13:29 2024 -0700
[SPARK-48744][CORE] Log entry should be constructed only once
### What changes were proposed in this pull request?
In the current implementation:
```
class LogEntry(messageWithContext: => MessageWithContext) {
def message: String = messageWithContext.message
def context: java.util.HashMap[String, String] =
messageWithContext.context
}
def logInfo(entry: LogEntry): Unit = {
if (log.isInfoEnabled) {
withLogContext(entry.context) {
log.info(entry.message)
}
}
}
```
The field `messageWithContext` is constructed twice, one from
`entry.context` and another one from `entry.message`.
This PR is to improve this and ensure a log entry is constructed only once.
### Why are the changes needed?
Improve the performance of logging
### Does this PR introduce _any_ user-facing change?
No
### How was this patch tested?
New UT
### Was this patch authored or co-authored using generative AI tooling?
No
Closes #47135 from gengliangwang/addCache.
Authored-by: Gengliang Wang <[email protected]>
Signed-off-by: Gengliang Wang <[email protected]>
---
.../scala/org/apache/spark/internal/Logging.scala | 6 ++--
.../apache/spark/util/StructuredLoggingSuite.scala | 33 +++++++++++++++++++++-
2 files changed, 36 insertions(+), 3 deletions(-)
diff --git
a/common/utils/src/main/scala/org/apache/spark/internal/Logging.scala
b/common/utils/src/main/scala/org/apache/spark/internal/Logging.scala
index 1d43bda6e4fc..8eea9b44da26 100644
--- a/common/utils/src/main/scala/org/apache/spark/internal/Logging.scala
+++ b/common/utils/src/main/scala/org/apache/spark/internal/Logging.scala
@@ -99,9 +99,11 @@ case class MessageWithContext(message: String, context:
java.util.HashMap[String
* Companion class for lazy evaluation of the MessageWithContext instance.
*/
class LogEntry(messageWithContext: => MessageWithContext) {
- def message: String = messageWithContext.message
+ private lazy val cachedMessageWithContext: MessageWithContext =
messageWithContext
- def context: java.util.HashMap[String, String] = messageWithContext.context
+ def message: String = cachedMessageWithContext.message
+
+ def context: java.util.HashMap[String, String] =
cachedMessageWithContext.context
}
/**
diff --git
a/common/utils/src/test/scala/org/apache/spark/util/StructuredLoggingSuite.scala
b/common/utils/src/test/scala/org/apache/spark/util/StructuredLoggingSuite.scala
index 52380b2edc7c..b3e103f46337 100644
---
a/common/utils/src/test/scala/org/apache/spark/util/StructuredLoggingSuite.scala
+++
b/common/utils/src/test/scala/org/apache/spark/util/StructuredLoggingSuite.scala
@@ -25,7 +25,7 @@ import com.fasterxml.jackson.module.scala.DefaultScalaModule
import org.apache.logging.log4j.Level
import org.scalatest.funsuite.AnyFunSuite // scalastyle:ignore funsuite
-import org.apache.spark.internal.{LogEntry, Logging, LogKey, LogKeys, MDC}
+import org.apache.spark.internal.{LogEntry, Logging, LogKey, LogKeys, MDC,
MessageWithContext}
trait LoggingSuiteBase
extends AnyFunSuite // scalastyle:ignore funsuite
@@ -228,6 +228,37 @@ trait LoggingSuiteBase
verifyMsgWithConcat(level, logOutput)
}
}
+
+ test("LogEntry should construct MessageWithContext only once") {
+ var constructionCount = 0
+
+ def constructMessageWithContext(): MessageWithContext = {
+ constructionCount += 1
+ log"Lost executor ${MDC(LogKeys.EXECUTOR_ID, "1")}."
+ }
+ logInfo(constructMessageWithContext())
+ assert(constructionCount === 1)
+ }
+
+ test("LogEntry should construct MessageWithContext only once II") {
+ var constructionCount = 0
+ var constructionCount2 = 0
+
+ def executorId(): String = {
+ constructionCount += 1
+ "1"
+ }
+
+ def workerId(): String = {
+ constructionCount2 += 1
+ "2"
+ }
+
+ logInfo(log"Lost executor ${MDC(LogKeys.EXECUTOR_ID, executorId())}." +
+ log"worker id ${MDC(LogKeys.WORKER_ID, workerId())}")
+ assert(constructionCount === 1)
+ assert(constructionCount2 === 1)
+ }
}
class StructuredLoggingSuite extends LoggingSuiteBase {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
