From 3da88b55c2d30d5fdd256615762e4d764923c140 Mon Sep 17 00:00:00 2001 From: kciesielski Date: Wed, 6 Mar 2024 14:12:14 +0100 Subject: [PATCH 1/5] Cache ContenType to avoid excessive parsing for Pekko responses --- .../server/pekkohttp/ContentTypeCache.scala | 28 +++++++++++++++++++ .../pekkohttp/PekkoToResponseBody.scala | 2 +- 2 files changed, 29 insertions(+), 1 deletion(-) create mode 100644 server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/ContentTypeCache.scala diff --git a/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/ContentTypeCache.scala b/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/ContentTypeCache.scala new file mode 100644 index 0000000000..9571770181 --- /dev/null +++ b/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/ContentTypeCache.scala @@ -0,0 +1,28 @@ +package sttp.tapir.server.pekkohttp + +import org.apache.pekko.http.scaladsl.model.ContentType +import scala.collection.concurrent.TrieMap + +/** Pekko-specific ConentType has to be created if an endpoint overrides it, but we want to reduce overhead of the expensive + * ContentType.parse operation if possible. Parsing may also happen for cases not listed explictly in + * PekkoToResponseBody.formatToContentType. This cache doesn't have to save atomically, because the worst case scenario is that we parse he + * same header a few times before it's saved. The cache is not cleared, because the number of different content types is limited and the + * cache is not expected to grow too much. The only exception is when there is a boundary in the header, but in such situation the endpoint + * contentType shouldn't be overriden. Just in case this happens, we limit the cache size. + */ +private[pekkohttp] object ContentTypeCache { + private val cache = TrieMap[String, ContentType]() + private val Limit = 100 + + def getOrParse(headerValue: String): ContentType = { + cache.get(headerValue) match { + case Some(contentType) => + contentType + case None => + val contentType = + ContentType.parse(headerValue).getOrElse(throw new IllegalArgumentException(s"Cannot parse content type: $headerValue")) + if (cache.size <= Limit) cache.putIfAbsent(headerValue, contentType) else () + contentType + } + } +} diff --git a/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/PekkoToResponseBody.scala b/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/PekkoToResponseBody.scala index c355140ffc..5cbd2045d7 100644 --- a/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/PekkoToResponseBody.scala +++ b/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/PekkoToResponseBody.scala @@ -132,7 +132,7 @@ private[pekkohttp] class PekkoToResponseBody(implicit m: Materializer, ec: Execu } private def parseContentType(ct: String): ContentType = - ContentType.parse(ct).getOrElse(throw new IllegalArgumentException(s"Cannot parse content type: $ct")) + ContentTypeCache.getOrParse(ct) private def charsetToHttpCharset(charset: Charset): HttpCharset = HttpCharset.custom(charset.name()) From bb4ea8d6ba2a39f0eec0229684556a5be0fb534a Mon Sep 17 00:00:00 2001 From: kciesielski Date: Wed, 6 Mar 2024 16:07:06 +0100 Subject: [PATCH 2/5] Filter headers before parsing --- .../src/main/scala/sttp/tapir/server/pekkohttp/PekkoModel.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/PekkoModel.scala b/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/PekkoModel.scala index c997e1d351..66b5fdd445 100644 --- a/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/PekkoModel.scala +++ b/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/PekkoModel.scala @@ -13,8 +13,8 @@ private[pekkohttp] object PekkoModel { def parseHeadersOrThrowWithoutContentHeaders(hs: HasHeaders): Seq[HttpHeader] = hs.headers - .map(parseHeaderOrThrow) .filterNot(h => h.is(ctHeaderNameLowerCase) || h.is(clHeaderNameLowerCase) || h.is(teHeaderNameLowerCase)) + .map(parseHeaderOrThrow) def parseHeaderOrThrow(h: Header): HttpHeader = HttpHeader.parse(h.name, h.value) match { From faada28e3cc89995976b6b672b664d4352d1360a Mon Sep 17 00:00:00 2001 From: kciesielski Date: Wed, 6 Mar 2024 16:41:13 +0100 Subject: [PATCH 3/5] Apply optimizations to akka-http --- .../src/main/scala/sttp/tapir/server/akkahttp/AkkaModel.scala | 2 +- .../scala/sttp/tapir/server/akkahttp/AkkaToResponseBody.scala | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/AkkaModel.scala b/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/AkkaModel.scala index 66527fcb8c..f37162ee8c 100644 --- a/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/AkkaModel.scala +++ b/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/AkkaModel.scala @@ -13,8 +13,8 @@ private[akkahttp] object AkkaModel { def parseHeadersOrThrowWithoutContentHeaders(hs: HasHeaders): Seq[HttpHeader] = hs.headers - .map(parseHeaderOrThrow) .filterNot(h => h.is(ctHeaderNameLowerCase) || h.is(clHeaderNameLowerCase) || h.is(teHeaderNameLowerCase)) + .map(parseHeaderOrThrow) def parseHeaderOrThrow(h: Header): HttpHeader = HttpHeader.parse(h.name, h.value) match { diff --git a/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/AkkaToResponseBody.scala b/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/AkkaToResponseBody.scala index 10d3e752ba..3e7d7e0833 100644 --- a/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/AkkaToResponseBody.scala +++ b/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/AkkaToResponseBody.scala @@ -132,7 +132,7 @@ private[akkahttp] class AkkaToResponseBody(implicit m: Materializer, ec: Executi } private def parseContentType(ct: String): ContentType = - ContentType.parse(ct).getOrElse(throw new IllegalArgumentException(s"Cannot parse content type: $ct")) + ContentTypeCache.getOrParse(ct) private def charsetToHttpCharset(charset: Charset): HttpCharset = HttpCharset.custom(charset.name()) From 04e9804ae788f8f537828a7c99f9eef7f378f7a7 Mon Sep 17 00:00:00 2001 From: kciesielski Date: Wed, 6 Mar 2024 16:57:04 +0100 Subject: [PATCH 4/5] Add missing class for akka-http --- .../server/akkahttp/ContentTypeCache.scala | 28 +++++++++++++++++++ 1 file changed, 28 insertions(+) create mode 100644 server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/ContentTypeCache.scala diff --git a/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/ContentTypeCache.scala b/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/ContentTypeCache.scala new file mode 100644 index 0000000000..7e9f105c86 --- /dev/null +++ b/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/ContentTypeCache.scala @@ -0,0 +1,28 @@ +package sttp.tapir.server.akkahttp + +import akka.http.scaladsl.model.ContentType +import scala.collection.concurrent.TrieMap + +/** Pekko-specific ConentType has to be created if an endpoint overrides it, but we want to reduce overhead of the expensive + * ContentType.parse operation if possible. Parsing may also happen for cases not listed explictly in + * PekkoToResponseBody.formatToContentType. This cache doesn't have to save atomically, because the worst case scenario is that we parse he + * same header a few times before it's saved. The cache is not cleared, because the number of different content types is limited and the + * cache is not expected to grow too much. The only exception is when there is a boundary in the header, but in such situation the endpoint + * contentType shouldn't be overriden. Just in case this happens, we limit the cache size. + */ +private[akkahttp] object ContentTypeCache { + private val cache = TrieMap[String, ContentType]() + private val Limit = 100 + + def getOrParse(headerValue: String): ContentType = { + cache.get(headerValue) match { + case Some(contentType) => + contentType + case None => + val contentType = + ContentType.parse(headerValue).getOrElse(throw new IllegalArgumentException(s"Cannot parse content type: $headerValue")) + if (cache.size <= Limit) cache.putIfAbsent(headerValue, contentType) else () + contentType + } + } +} From 2415d1deac08fb3fc5f86a5e04cb891f399e0acc Mon Sep 17 00:00:00 2001 From: kciesielski Date: Thu, 7 Mar 2024 11:01:29 +0100 Subject: [PATCH 5/5] Don't cache MediaType with params --- .../scala/sttp/tapir/server/akkahttp/ContentTypeCache.scala | 3 ++- .../scala/sttp/tapir/server/pekkohttp/ContentTypeCache.scala | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/ContentTypeCache.scala b/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/ContentTypeCache.scala index 7e9f105c86..cd00ac12ca 100644 --- a/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/ContentTypeCache.scala +++ b/server/akka-http-server/src/main/scala/sttp/tapir/server/akkahttp/ContentTypeCache.scala @@ -21,7 +21,8 @@ private[akkahttp] object ContentTypeCache { case None => val contentType = ContentType.parse(headerValue).getOrElse(throw new IllegalArgumentException(s"Cannot parse content type: $headerValue")) - if (cache.size <= Limit) cache.putIfAbsent(headerValue, contentType) else () + // We don't want to fill the cache with parameterized media types (BTW charset does not appear in params) + val _ = if (cache.size <= Limit && contentType.mediaType.params.isEmpty) cache.putIfAbsent(headerValue, contentType) contentType } } diff --git a/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/ContentTypeCache.scala b/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/ContentTypeCache.scala index 9571770181..12ef2cac78 100644 --- a/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/ContentTypeCache.scala +++ b/server/pekko-http-server/src/main/scala/sttp/tapir/server/pekkohttp/ContentTypeCache.scala @@ -21,7 +21,8 @@ private[pekkohttp] object ContentTypeCache { case None => val contentType = ContentType.parse(headerValue).getOrElse(throw new IllegalArgumentException(s"Cannot parse content type: $headerValue")) - if (cache.size <= Limit) cache.putIfAbsent(headerValue, contentType) else () + // We don't want to fill the cache with parameterized media types (BTW charset does not appear in params) + val _ = if (cache.size <= Limit && contentType.mediaType.params.isEmpty) cache.putIfAbsent(headerValue, contentType) contentType } }