spark-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From marmbrus <...@git.apache.org>
Subject [GitHub] spark pull request: remove scalalogging-slf4j dependency
Date Sun, 06 Apr 2014 18:26:28 GMT
Github user marmbrus commented on the pull request:

    https://github.com/apache/spark/pull/332#issuecomment-39675896
  
    Its not a matter of the if statements inside of spark logging code.  Scala logging rewrites
your log statements to put the if checks _inline_ with _every_ logging statement.
    
    You write: `logger.debug(s"Log message $someExpensiveComputation")`
    You get:
    ```
    if(logger.debugEnabled) {
      val logMsg = "Log message" + someExpensiveComputation()
      logger.debug(logMessage)
    }
    ```
    
    Sparks code does something like this:
    ```
    val logMessage = new Function0() { def call() =  "Log message" + someExpensiveComputation()
}
    log.debug(logMessage)
    ```
    The macros allow you to avoid both the function call, and possible gc overhead of allocating
the closure in cases where the log does not fire.
    
    While not huge, this does make a performance difference in practice:
    ```
    std logging: 19885.48ms
    spark logging 914.408ms
    scala logging 729.779ms
    ```
    
    (And this was only a micro benchmark with no GC pressure).
    
    Given that this is library is a thin layer, endorsed by typesafe, that integrates nicely
with our current logging framework (and is easily pluggable if we ever decide to change),
and improves performance, I really think this PR is a step backwards.  If we want consistency
we should probably instead get rid of Sparks home grown logging code.
    



---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

Mime
View raw message