From 21c8ae5e8bd2605913aa0d7a026d800d0011c439 Mon Sep 17 00:00:00 2001 From: kyri-petrou <67301607+kyri-petrou@users.noreply.github.com> Date: Thu, 18 Apr 2024 14:24:42 +1000 Subject: [PATCH] Add the `Date` header to responses by default (#2773) * Add the `Date` header to responses by default * Don't add header for 1xx or 5xx responses * Update GH workflow * fmt --- .github/workflows/ci.yml | 43 +++++++++++++++- .../CachedDateHeaderBenchmark.scala | 27 ++++++++++ .../zio/http/netty/CachedDateHeader.scala | 51 +++++++++++++++++++ .../zio/http/netty/NettyResponseEncoder.scala | 24 +++++++-- .../zio/http/netty/CachedDateHeaderSpec.scala | 36 +++++++++++++ 5 files changed, 175 insertions(+), 6 deletions(-) create mode 100644 zio-http-benchmarks/src/main/scala/zhttp.benchmarks/CachedDateHeaderBenchmark.scala create mode 100644 zio-http/jvm/src/main/scala/zio/http/netty/CachedDateHeader.scala create mode 100644 zio-http/jvm/src/test/scala/zio/http/netty/CachedDateHeaderSpec.scala diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e2da08e3ad..d6f91f0e3e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -209,6 +209,40 @@ jobs: id: push_codecov run: 'bash <(curl -s https://codecov.io/bash)' + Jmh_CachedDateHeaderBenchmark: + name: Jmh CachedDateHeaderBenchmark + if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} + strategy: + matrix: + os: [ubuntu-latest] + scala: [2.13.12] + java: [temurin@8] + runs-on: ${{ matrix.os }} + steps: + - uses: actions/checkout@v2 + with: + path: zio-http + + - uses: actions/setup-java@v2 + with: + distribution: temurin + java-version: 11 + + - name: Benchmark_Main + id: Benchmark_Main + env: + GITHUB_TOKEN: ${{secrets.ACTIONS_PAT}} + run: | + cd zio-http + sed -i -e '$aaddSbtPlugin("pl.project13.scala" % "sbt-jmh" % "0.4.3")' project/plugins.sbt + cat > Main_CachedDateHeaderBenchmark.txt + sbt -no-colors -v "zioHttpBenchmarks/jmh:run -i 3 -wi 3 -f1 -t1 CachedDateHeaderBenchmark" | grep -e "thrpt" -e "avgt" >> ../Main_CachedDateHeaderBenchmark.txt + + - uses: actions/upload-artifact@v3 + with: + name: Jmh_Main_CachedDateHeaderBenchmark + path: Main_CachedDateHeaderBenchmark.txt + Jmh_CookieDecodeBenchmark: name: Jmh CookieDecodeBenchmark if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} @@ -551,7 +585,7 @@ jobs: Jmh_cache: name: Cache Jmh benchmarks - needs: [Jmh_CookieDecodeBenchmark, Jmh_EndpointBenchmark, Jmh_HttpCollectEval, Jmh_HttpCombineEval, Jmh_HttpNestedFlatMapEval, Jmh_HttpRouteTextPerf, Jmh_ProbeContentTypeBenchmark, Jmh_SchemeDecodeBenchmark, Jmh_ServerInboundHandlerBenchmark, Jmh_UtilBenchmark] + needs: [Jmh_CachedDateHeaderBenchmark, Jmh_CookieDecodeBenchmark, Jmh_EndpointBenchmark, Jmh_HttpCollectEval, Jmh_HttpCombineEval, Jmh_HttpNestedFlatMapEval, Jmh_HttpRouteTextPerf, Jmh_ProbeContentTypeBenchmark, Jmh_SchemeDecodeBenchmark, Jmh_ServerInboundHandlerBenchmark, Jmh_UtilBenchmark] if: ${{ github.event_name == 'push' && github.ref == 'refs/heads/main' }} strategy: matrix: @@ -560,6 +594,13 @@ jobs: java: [temurin@8] runs-on: ${{ matrix.os }} steps: + - uses: actions/download-artifact@v3 + with: + name: Jmh_Main_CachedDateHeaderBenchmark + + - name: Format_Main_CachedDateHeaderBenchmark + run: cat Main_CachedDateHeaderBenchmark.txt >> Main_benchmarks.txt + - uses: actions/download-artifact@v3 with: name: Jmh_Main_CookieDecodeBenchmark diff --git a/zio-http-benchmarks/src/main/scala/zhttp.benchmarks/CachedDateHeaderBenchmark.scala b/zio-http-benchmarks/src/main/scala/zhttp.benchmarks/CachedDateHeaderBenchmark.scala new file mode 100644 index 0000000000..152ff6c88c --- /dev/null +++ b/zio-http-benchmarks/src/main/scala/zhttp.benchmarks/CachedDateHeaderBenchmark.scala @@ -0,0 +1,27 @@ +package zio.http.netty + +import java.time.ZonedDateTime +import java.util.concurrent.TimeUnit + +import zio.http.internal.DateEncoding + +import org.openjdk.jmh.annotations._ + +@State(Scope.Benchmark) +@BenchmarkMode(Array(Mode.AverageTime)) +@OutputTimeUnit(TimeUnit.NANOSECONDS) +@Threads(16) +@Fork(1) +@Warmup(iterations = 3, time = 1) +@Measurement(iterations = 5, time = 1) +class CachedDateHeaderBenchmark { + private val dateHeaderCache = new CachedDateHeader() + + @Benchmark + def benchmarkCached() = + dateHeaderCache.get() + + @Benchmark + def benchmarkFresh() = + DateEncoding.default.encodeDate(ZonedDateTime.now()) +} diff --git a/zio-http/jvm/src/main/scala/zio/http/netty/CachedDateHeader.scala b/zio-http/jvm/src/main/scala/zio/http/netty/CachedDateHeader.scala new file mode 100644 index 0000000000..63171b2397 --- /dev/null +++ b/zio-http/jvm/src/main/scala/zio/http/netty/CachedDateHeader.scala @@ -0,0 +1,51 @@ +package zio.http.netty + +import java.time.{Clock, LocalDateTime, ZoneOffset} +import java.util.concurrent.locks.LockSupport + +import zio.http.internal.DateEncoding + +private object CachedDateHeader { + lazy val default: CachedDateHeader = new CachedDateHeader() +} + +final private class CachedDateHeader( + clock: Clock = Clock.tickSeconds(ZoneOffset.UTC), + dateEncoding: DateEncoding = DateEncoding.default, +) { + private var _headerValue = renderDateHeaderValue(clock.millis()) + + { + val t = new Ticker + t.setDaemon(true) + t.setName(s"zio.http.netty.DateHeaderEncoder.Scheduler") + t.setPriority(Thread.MAX_PRIORITY) + t.start() + } + + def get(): String = _headerValue + + private final class Ticker extends Thread { + override def run(): Unit = { + val clock0 = clock + var currentMillis = clock0.millis() + while (!isInterrupted) { + LockSupport.parkUntil(currentMillis + 1000) + currentMillis = clock0.millis() + updateHeaderValue(currentMillis) + } + } + } + + private def renderDateHeaderValue(epochMilli: Long): String = { + val dt = LocalDateTime + .ofEpochSecond(epochMilli / 1000L, 0, ZoneOffset.UTC) + .atZone(ZoneOffset.UTC) + + dateEncoding.encodeDate(dt) + } + + private def updateHeaderValue(epochMilli: Long): Unit = + _headerValue = renderDateHeaderValue(epochMilli) + +} diff --git a/zio-http/jvm/src/main/scala/zio/http/netty/NettyResponseEncoder.scala b/zio-http/jvm/src/main/scala/zio/http/netty/NettyResponseEncoder.scala index 6da366db16..5800600911 100644 --- a/zio-http/jvm/src/main/scala/zio/http/netty/NettyResponseEncoder.scala +++ b/zio-http/jvm/src/main/scala/zio/http/netty/NettyResponseEncoder.scala @@ -27,6 +27,7 @@ import io.netty.channel.ChannelHandlerContext import io.netty.handler.codec.http._ private[zio] object NettyResponseEncoder { + private val dateHeaderCache = CachedDateHeader.default def encode( ctx: ChannelHandlerContext, @@ -38,8 +39,10 @@ private[zio] object NettyResponseEncoder { val bytes = runtime.runtime(ctx).unsafe.run(body.asArray).getOrThrow() fastEncode(response, bytes) } else { + val status = response.status val jHeaders = Conversions.headersToNetty(response.headers) - val jStatus = Conversions.statusToNetty(response.status) + val jStatus = Conversions.statusToNetty(status) + maybeAddDateHeader(jHeaders, status) response.body.knownContentLength match { case Some(contentLength) if !jHeaders.contains(HttpHeaderNames.CONTENT_LENGTH) => @@ -63,16 +66,27 @@ private[zio] object NettyResponseEncoder { private def doEncode(response: Response, bytes: Array[Byte]): FullHttpResponse = { val jHeaders = Conversions.headersToNetty(response.headers) val hasContentLength = jHeaders.contains(HttpHeaderNames.CONTENT_LENGTH) + val status = response.status + maybeAddDateHeader(jHeaders, status) val jStatus = Conversions.statusToNetty(response.status) - val jContent = Unpooled.wrappedBuffer(bytes) - val jResponse = new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, jStatus, jContent, false) + val jContent = Unpooled.wrappedBuffer(bytes) if (!hasContentLength) jHeaders.set(HttpHeaderNames.CONTENT_LENGTH, jContent.readableBytes()) - jResponse.headers().add(jHeaders) - jResponse + new DefaultFullHttpResponse(HttpVersion.HTTP_1_1, jStatus, jContent, jHeaders, EmptyHttpHeaders.INSTANCE) + } + + /** + * We don't need to add the Date header in the following case: + * - Status code is 1xx + * - Status code is 5xx + * - User already provided it + */ + private def maybeAddDateHeader(headers: HttpHeaders, status: Status): Unit = { + if (status.isInformational || status.isServerError || headers.contains(HttpHeaderNames.DATE)) () + else headers.set(HttpHeaderNames.DATE, dateHeaderCache.get()) } } diff --git a/zio-http/jvm/src/test/scala/zio/http/netty/CachedDateHeaderSpec.scala b/zio-http/jvm/src/test/scala/zio/http/netty/CachedDateHeaderSpec.scala new file mode 100644 index 0000000000..a624c4c806 --- /dev/null +++ b/zio-http/jvm/src/test/scala/zio/http/netty/CachedDateHeaderSpec.scala @@ -0,0 +1,36 @@ +package zio.http.netty + +import java.time.ZonedDateTime + +import zio._ +import zio.test.{Spec, TestAspect, TestEnvironment, assertCompletes} + +import zio.http.ZIOHttpSpec +import zio.http.internal.DateEncoding + +object CachedDateHeaderSpec extends ZIOHttpSpec { + private val dateHeaderCache = CachedDateHeader.default + + override def spec: Spec[TestEnvironment with Scope, Any] = + suite("CachedDateHeader")( + test("yields the same date header value as DateEncoding") { + val f = ZIO.suspendSucceed { + val uncached = DateEncoding.default.encodeDate(ZonedDateTime.now()) + val cached = dateHeaderCache.get() + + ZIO + .fail( + new Exception( + s"Mismatch in cached and uncached date header value:\n\tuncached: $uncached\n\tcached: $cached", + ), + ) + .unless(uncached == cached) + } + .delay(5.milli) + .retryN(1) + + ZIO.foreachDiscard((1 to 300).toList)(_ => f) *> assertCompletes + } + @@ TestAspect.withLiveClock, + ) +}