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

Utilize ZIO / ZQuery eager constructors and accessors wherever possible #2297

Merged
merged 5 commits into from
Jun 25, 2024

Conversation

kyri-petrou
Copy link
Collaborator

NOTE: PR is going to fail because the zio-query dependency is a Snapshot version. I'll update this PR once I realise zio-query v0.7.2

This PR better utilizes ZIO and ZQuery eager methods and accessors in the following ways:

  1. Use Exit.succeed wherever we don't need to capture side-effects or recurse in a method
  2. In Wrappers: prefer using standard if statements to avoid lazy evaluation and extra allocations
  3. In Executor: Attempt to extract an Exit from a ZQuery, and evaluate it as a pure value if it's possible

I'd like to elaborate a bit more on (3). In the GQL applications I have at $WORK, we often define our types like this:

case class Author(
  name: Query[String],
  age: Query[Int],
)

Depending on some logic that handles query composition, in some cases we already know the name and age. So we can do this:

Author(ZQuery.succeedNow("John"), ZQuery.succeedNow(55))

In such cases, Caliban can treat these query fields as pure values and reduce the field as if the type didn't contain any queries.

I run some benchmarks comparing the performance of using ZQuery.succeed vs ZQuery.succeedNow on some of the fields on MultiFieldEntity:

[info] Benchmark                                            (execution)  (size)   Mode  Cnt      Score      Error  Units
[info] NestedZQueryBenchmark.multifieldQueryBenchmark        sequential     100  thrpt    5   9664.459 ±   43.892  ops/s
[info] NestedZQueryBenchmark.multifieldQueryBenchmark        sequential   10000  thrpt    5     90.368 ±   21.104  ops/s
[info] NestedZQueryBenchmark.multifieldQueryBenchmark          parallel     100  thrpt    5   4551.865 ±   10.065  ops/s
[info] NestedZQueryBenchmark.multifieldQueryBenchmark          parallel   10000  thrpt    5     55.528 ±    1.476  ops/s
[info] NestedZQueryBenchmark.multifieldQueryBenchmark           batched     100  thrpt    5   9371.414 ±  488.100  ops/s
[info] NestedZQueryBenchmark.multifieldQueryBenchmark           batched   10000  thrpt    5     87.193 ±    8.166  ops/s
[info] NestedZQueryBenchmark.multifieldQueryEagerBenchmark   sequential     100  thrpt    5  17530.337 ±  447.433  ops/s
[info] NestedZQueryBenchmark.multifieldQueryEagerBenchmark   sequential   10000  thrpt    5    175.372 ±    9.420  ops/s
[info] NestedZQueryBenchmark.multifieldQueryEagerBenchmark     parallel     100  thrpt    5   7452.806 ±  701.639  ops/s
[info] NestedZQueryBenchmark.multifieldQueryEagerBenchmark     parallel   10000  thrpt    5     98.636 ±   12.712  ops/s
[info] NestedZQueryBenchmark.multifieldQueryEagerBenchmark      batched     100  thrpt    5  17380.816 ± 2332.441  ops/s
[info] NestedZQueryBenchmark.multifieldQueryEagerBenchmark      batched   10000  thrpt    5    179.397 ±   19.380  ops/s

Even if we were not able to fully reduce the object (since it still contains 1 field that uses ZQuery.succeed) we managed to double our throughput by extracting the values from the fields that were eagerly constructed.

@@ -125,7 +125,7 @@ package object tapir {
val replacedArgs = args.map { case (k, v) => reverseArgNames.getOrElse(k, k) -> v }
QueryStep(
ZQuery
.fromZIO(ZIO.fromEither(argBuilder.build(InputValue.ObjectValue(replacedArgs))))
.fromZIO(Exit.fromEither(argBuilder.build(InputValue.ObjectValue(replacedArgs))))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's no ZQuery.fromEither btw?

Copy link
Collaborator Author

@kyri-petrou kyri-petrou Jun 23, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah yeah I'll change it

@kyri-petrou kyri-petrou merged commit cf93e97 into series/2.x Jun 25, 2024
11 checks passed
@kyri-petrou kyri-petrou deleted the embrace-eagerness branch June 25, 2024 02:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants