zio-http icon indicating copy to clipboard operation
zio-http copied to clipboard

Convert headers from case class to sealed trait

Open jgoday opened this issue 2 years ago • 3 comments

Attempt to implement #1007.

Headers ADT's are:

case object EmptyHeaders extends Headers
case class HeadersFromChunk(value: Chunk[Header]) extends Headers
case class HeadersFromHttp(value: HttpHeaders) extends Headers
case class UpdatedHeaders(update: Headers => Headers, value: Headers) extends Headers
case class ModifyHeaders(modify: Header => Header, value: Headers) extends Headers
case class HeadersCons(a: Headers, b: Headers) extends Headers

Added some naive/basic benchmark (HttpHeadersBenchmark), adding and removing some headers:

    val _ = Headers.empty
      .addHeader("Content-Type", "application/json")
      .addHeader("Content-Length", "0")
      .addHeader("Accepts", "application/json")
      .withContentEncoding("utf-8")
      .removeHeader("Accepts")
  • Case class headers: 1597240,295 ops/sec
  • Sealed trait headers: 8030336,433 ops/sec

full output goes below

Current case class headers:

[info] # JMH version: 1.32
[info] # VM version: JDK 17, OpenJDK 64-Bit Server VM, 17+35-2724
[info] # VM invoker: /Users/jgoday/Library/Caches/Coursier/jvm/[email protected]/Contents/Home/bin/java
[info] # VM options: -Dio.netty.leakDetectionLevel=paranoid -DZHttpLogLevel=INFO
[info] # Blackhole mode: full + dont-inline hint
[info] # Warmup: 3 iterations, 10 s each
[info] # Measurement: 3 iterations, 10 s each
[info] # Timeout: 10 min per iteration
[info] # Threads: 1 thread, will synchronize iterations
[info] # Benchmark mode: Throughput, ops/time
[info] # Benchmark: zhttp.benchmarks.HttpHeadersBenchmark.benchmarkApp
[info] # Run progress: 0,00% complete, ETA 00:01:00
[info] # Fork: 1 of 1
[info] # Warmup Iteration   1: 1486934,006 ops/s
[info] # Warmup Iteration   2: 1637439,246 ops/s
[info] # Warmup Iteration   3: 1636375,782 ops/s
[info] Iteration   1: 1640994,184 ops/s
[info] Iteration   2: 1537598,331 ops/s
[info] Iteration   3: 1613128,370 ops/s
[info] Result "zhttp.benchmarks.HttpHeadersBenchmark.benchmarkApp":
[info]   1597240,295 ±(99.9%) 975996,330 ops/s [Average]
[info]   (min, avg, max) = (1537598,331, 1597240,295, 1640994,184), stdev = 53497,652
[info]   CI (99.9%): [621243,965, 2573236,625] (assumes normal distribution)
[info] # Run complete. Total time: 00:01:00
[info] REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on
[info] why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial
[info] experiments, perform baseline and negative tests that provide experimental control, make sure
[info] the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts.
[info] Do not assume the numbers tell you what you want them to tell.
[info] Benchmark                           Mode  Cnt        Score        Error  Units
[info] HttpHeadersBenchmark.benchmarkApp  thrpt    3  1597240,295 ± 975996,330  ops/s
[success] Total time: 89 s (01:29), completed 27 jul 2022 0:27:43

Sealed trait headers:

[info] # JMH version: 1.32
[info] # VM version: JDK 17, OpenJDK 64-Bit Server VM, 17+35-2724
[info] # VM invoker: /Users/jgoday/Library/Caches/Coursier/jvm/[email protected]/Contents/Home/bin/java
[info] # VM options: -Dio.netty.leakDetectionLevel=paranoid -DZHttpLogLevel=INFO
[info] # Blackhole mode: full + dont-inline hint
[info] # Warmup: 3 iterations, 10 s each
[info] # Measurement: 3 iterations, 10 s each
[info] # Timeout: 10 min per iteration
[info] # Threads: 1 thread, will synchronize iterations
[info] # Benchmark mode: Throughput, ops/time
[info] # Benchmark: zhttp.benchmarks.HttpHeadersBenchmark.benchmarkApp
[info] # Run progress: 0,00% complete, ETA 00:01:00
[info] # Fork: 1 of 1
[info] # Warmup Iteration   1: 7536276,582 ops/s
[info] # Warmup Iteration   2: 7992448,592 ops/s
[info] # Warmup Iteration   3: 8060069,362 ops/s
[info] Iteration   1: 8011678,794 ops/s
[info] Iteration   2: 8016804,008 ops/s
[info] Iteration   3: 8062526,497 ops/s
[info] Result "zhttp.benchmarks.HttpHeadersBenchmark.benchmarkApp":
[info]   8030336,433 ±(99.9%) 510732,071 ops/s [Average]
[info]   (min, avg, max) = (8011678,794, 8030336,433, 8062526,497), stdev = 27994,948
[info]   CI (99.9%): [7519604,362, 8541068,504] (assumes normal distribution)
[info] # Run complete. Total time: 00:01:00
[info] REMEMBER: The numbers below are just data. To gain reusable insights, you need to follow up on
[info] why the numbers are the way they are. Use profilers (see -prof, -lprof), design factorial
[info] experiments, perform baseline and negative tests that provide experimental control, make sure
[info] the benchmarking environment is safe on JVM/OS/HW level, ask for reviews from the domain experts.
[info] Do not assume the numbers tell you what you want them to tell.
[info] Benchmark                           Mode  Cnt        Score        Error  Units
[info] HttpHeadersBenchmark.benchmarkApp  thrpt    3  8030336,433 ± 510732,071  ops/s
[success] Total time: 82 s (01:22), completed 27 jul 2022 0:22:10

jgoday avatar Jul 26 '22 22:07 jgoday

Codecov Report

Merging #1371 (c57b8c2) into main (66b571c) will increase coverage by 0.09%. The diff coverage is 77.77%.

@@            Coverage Diff             @@
##             main    #1371      +/-   ##
==========================================
+ Coverage   60.79%   60.89%   +0.09%     
==========================================
  Files          71       71              
  Lines        2482     2493      +11     
  Branches       84       77       -7     
==========================================
+ Hits         1509     1518       +9     
- Misses        973      975       +2     
Impacted Files Coverage Δ
.../main/scala/zhttp/http/headers/HeaderGetters.scala 34.14% <ø> (+0.81%) :arrow_up:
zio-http/src/main/scala/zhttp/http/Headers.scala 78.12% <77.77%> (-8.24%) :arrow_down:
...ttp/src/main/scala/zhttp/service/HttpRuntime.scala 86.20% <0.00%> (-0.46%) :arrow_down:
zio-http/src/main/scala/zhttp/service/Client.scala 92.18% <0.00%> (ø)
...ttp/src/main/scala/zhttp/http/middleware/Web.scala 82.92% <0.00%> (+0.87%) :arrow_up:
...main/scala/zhttp/http/headers/HeaderModifier.scala 19.76% <0.00%> (+1.16%) :arrow_up:

Help us with your feedback. Take ten seconds to tell us how you rate us.

codecov-commenter avatar Jul 26 '22 22:07 codecov-commenter

@jgoday the "write" path has been improved as the benchmark is showing but I think that there is an impact on the "read" path, toList is called everytime when we want to get a header. Can you add a jmh benchmark for the read path?

gciuloaica avatar Jul 28 '22 12:07 gciuloaica

@jgoday the "write" path has been improved as the benchmark is showing but I think that there is an impact on the "read" path, toList is called everytime when we want to get a header. Can you add a jmh benchmark for the read path?

@gciuloaica Fair enough :)

I just updated (allowing each sum type to override the HeaderGetters.header method) and added a new jmh benchmark to read and get specific headers, both constructed from netty.HttpHeaders and from a tuple iterator.

These are the results before and after the changes, note that with the first attempt, read benchmark results are very similiar (both uses toLists I think):

  • Original
HttpReadHeadersBenchmark.benchmarkApp  thrpt    3  4441062,801 ± 73269,884  ops/s
  • First attempt (based on HeaderGetters.header method)
HttpReadHeadersBenchmark.benchmarkApp  thrpt    3  4366901,517 ± 633754,413  ops/s
  • Last change (allowing header accessor override in each Header ADT sum type):
HttpReadHeadersBenchmark.benchmarkApp  thrpt    3  12623212,664 ± 251922,286  ops/s

Maybe a more realistic scenario is using Headers from netty HttpHeaders

  • Original:
HttpHeadersBenchmark.benchmarkHttpHeaders  thrpt    3  3131791,694 ± 295605,102  ops/s
  • Last change:
HttpHeadersBenchmark.benchmarkHttpHeaders  thrpt    3   7113061,854 ± 2448512,074  ops/s

jgoday avatar Jul 28 '22 18:07 jgoday

Closed by #1463. Thank you for your (parallel) work on this!

jdegoes avatar Sep 18 '22 14:09 jdegoes