uap-scala icon indicating copy to clipboard operation
uap-scala copied to clipboard

Experienced a memory leak

Open dgohlke opened this issue 5 years ago • 3 comments

I'm using uap-scala in a mobile backend that services 4.5 million requests a day. While recently debugging a memory leak, I discovered snakeyaml consuming an excessive amount of heap memory. org.yaml.snakeyaml.error.Mark was consuming 115 MB with nearly 3 million instances. Using https://github.com/jrudolph/sbt-dependency-graph, I found uap-scala was the only dependency in our build that was dependent on snakeyaml.

We have a playframework application setup with a logging filter similar to this (simplified):

import akka.util.ByteString
import javax.inject.Inject
import org.uaparser.scala.{Client,Parser}
import play.api.Logger
import play.api.libs.streams.Accumulator
import play.api.mvc._
import scala.concurrent.{ExecutionContext, Future}

class LoggingFilter @Inject() (implicit ec: ExecutionContext) extends EssentialFilter {

  def apply(nextFilter: EssentialAction): EssentialAction = new EssentialAction {
    def apply(request: RequestHeader): Accumulator[ByteString, Result] = {
      nextFilter(request).map { result =>
        logData(request, result)
        result
      }
    }
  }

  def logData(request: RequestHeader, result: Result): Future[Unit] = {
    Future {
      val client: Client = Parser.default.parse(request.headers.get("User-Agent").get)
      Logger.info(s"User Agent: ${client.userAgent}, Device: ${client.device}")
    }
  }
}

The above problem exists with the above code. Here's what we did to fix it:

class LoggingFilter @Inject() (implicit ec: ExecutionContext) extends EssentialFilter {
  val uaParser: Parser = Parser.default

  def apply(nextFilter: EssentialAction): EssentialAction = new EssentialAction {
    def apply(request: RequestHeader): Accumulator[ByteString, Result] = {
      nextFilter(request).map { result =>
        logData(request, result)
        result
      }
    }
  }

  def logData(request: RequestHeader, result: Result): Future[Unit] = {
    Future {
      val client: Client = uaParser.parse(request.headers.get("User-Agent").get)
      Logger.info(s"User Agent: ${client.userAgent}, Device: ${client.device}")
    }
  }
}

We moved the Parser.default instantiation up into the root of the class. Now we no longer have the memory leak.

I'm wondering if either the documentation can be updated to suggest following a pattern of instantiating then parsing, or if this can be handled from a code perspective. In fact, I believe this change might address the problem:

 object Parser {
+  val yaml = new Yaml(new SafeConstructor)
   def fromInputStream(source: InputStream): Try[Parser] = Try {
-    val yaml = new Yaml(new SafeConstructor)
     val javaConfig = yaml.load(source).asInstanceOf[JMap[String, JList[JMap[String, String]]]]
     val config = javaConfig.asScala.toMap.mapValues(_.asScala.toList.map(_.asScala.toMap.filterNot {
       case (_ , value) => value eq null

Thoughts? I tried to submit this via PR, but i lack the permission in your repo.

dgohlke avatar Sep 11 '18 18:09 dgohlke