Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add caching mechanism for the date header value #3303

Closed
wants to merge 1 commit into from

Conversation

jordiolivares
Copy link
Contributor

@jordiolivares jordiolivares commented Apr 4, 2020

I was upgrading the http4s code in the Techempower Benchmark and got curious to see where some of the bottlenecks are.

One of them was the fact that the HTTP Date header is always calculated and rendered for each request. This means that if you receive a lot of requests in a second each time you're going to render the Date header.

To solve this, this PR adds a simple caching mechanism that only renders the date header once in a given second, reusing the same value for all subsequent requests in that same moment.

Copy link
Member

@rossabaker rossabaker left a comment

Thanks. I think akka-http does something similar. Is there any benchmark evidence that this improves things? The implementation itself looks good.

@jordiolivares
Copy link
Contributor Author

jordiolivares commented Apr 14, 2020

Yes, I tested the improvement with a JMH benchmark and obtained these results:

[info] Benchmark                              Mode  Cnt        Score        Error  Units
[info] Http4sDateFormatting.http4sWay        thrpt    3  1769821.455 ±   6346.432  ops/s
[info] Http4sDateFormatting.simplyCachedWay  thrpt    3  8865904.658 ± 459379.561  ops/s

The benchmark code was this short snippet:

@State(Scope.Thread)
class Http4sDateFormatting {
  @Benchmark
  def http4sWay(): Unit = {
    val rr = new StringWriter(512)
    rr << Date.name << ": " << Instant.now << "\r\n"
  }

  val SimplyCachedRenderer: Renderer[Instant] =
    new Renderer[Instant] {
      private val dateFormat =
        DateTimeFormatter
          .ofPattern("EEE, dd MMM yyyy HH:mm:ss zzz")
          .withLocale(Locale.US)
          .withZone(ZoneId.of("GMT"))

      var cachedString: String = dateFormat.format(Instant.now())
      var currentTime: Long = Instant.now().getEpochSecond

      def format(instant: Instant): String = {
        if (instant.getEpochSecond != currentTime) {
          currentTime = instant.getEpochSecond
          cachedString = dateFormat.format(instant)
        }
        cachedString
      }

      override def render(writer: Writer, t: Instant): writer.type =
        writer << format(t)
    }

  @Benchmark
  def simplyCachedWay(): Unit = {
    val rr = new StringWriter(512)
    rr << Date.name << ": "
    rr.<<(Instant.now)(SimplyCachedRenderer)
    rr << "\r\n"
  }
}

The benchmarked code isn't exactly the same as it creates a new Renderer[Instant] object, but the principle is the same

Copy link
Member

@rossabaker rossabaker left a comment

The impact in the context of serving a full response will not be as stark as the multiplier in the benchmark, but it should make a positive difference, and it's clearly written.

We could also consider making this a functoin on the Date header itself, but we're stricter about referential transparency in core, and the F wrapper would give back some of the gain.

I think we should go with this, and cherry-pick it onto series/0.21.

@rossabaker rossabaker added the enhancement label Apr 20, 2020
@rossabaker rossabaker added this to the 0.21.4 milestone Apr 20, 2020
aeons
aeons approved these changes Apr 20, 2020
@rossabaker rossabaker added the retarget label Apr 20, 2020
@rossabaker
Copy link
Member

rossabaker commented Apr 20, 2020

Cherry-picked for release into 0.21.x.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement retarget
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants