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

Restore executor configuration for zio-grpc #443

Merged
merged 3 commits into from
Apr 15, 2024
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,10 +1,45 @@
package com.example.helloworld

import scalapb.zio_grpc.{ServerMain, ServiceList}
import scalapb.zio_grpc.{Server, ServerLayer, ServerMain, ServiceList}
import zio.ZLayer

import java.util.concurrent.Executors

object GreeterServer extends ServerMain {
def services: ServiceList[Any] = ServiceList.add(GreeterImpl)

// Default port is 9000
override def port = 50051

override def serverLive: ZLayer[Any, Throwable, Server] = {
val sb = builder

/**
* Allow customization of the Executor with two environment variables:
*
* <p>
* <ul>
* <li>JVM_EXECUTOR_TYPE: direct, workStealing, single, fixed, cached</li>
* <li>JVM_EXECUTOR_THREADS: integer value.</li>
* </ul>
* </p>
*
* The number of Executor Threads will default to the number of
* availableProcessors(). Only the workStealing and fixed executors will use
* this value.
*/
val threads = System.getenv("JVM_EXECUTOR_THREADS")
Copy link
Owner

Choose a reason for hiding this comment

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

In general, should this be rather GRPC_SERVER_CPUS? This is the variable we pass to servers so that they know how many will be available upfront. Or, will the JVM_EXECUTOR_THREADS correctly do that under the hood? Unless there is some JVM magic happening, the service will assume max CPUs on the host (e.g., 32), use a proper GC for that but then it'll get clamped at, e.g., 1. This may also be the case for other JVM-based implementations here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I don't think there's any magic, so it will use Runtime.getRuntime.availableProcessors. I just copied the way it was done in the fs2 one, it seems to be used that way in 4 different tests: https://github.com/search?q=repo%3ALesnyRumcajs%2Fgrpc_bench%20JVM_EXECUTOR_THREADS&type=code
Shall I change them all to use GRPC_SERVER_CPUS instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just changed it.

var i_threads = Runtime.getRuntime.availableProcessors
if (threads != null && !threads.isEmpty) i_threads = threads.toInt
val value = System.getenv.getOrDefault("JVM_EXECUTOR_TYPE", "workStealing")
value match {
case "direct" => sb.directExecutor
case "single" => sb.executor(Executors.newSingleThreadExecutor)
case "fixed" => sb.executor(Executors.newFixedThreadPool(i_threads))
case "workStealing" => sb.executor(Executors.newWorkStealingPool(i_threads))
case "cached" => sb.executor(Executors.newCachedThreadPool)
}

ServerLayer.fromServiceList(sb, services)
}
}
Loading