Copilot commented on code in PR #2242:
URL: https://github.com/apache/pekko/pull/2242#discussion_r2366108695
##########
actor/src/main/resources/reference.conf:
##########
@@ -569,6 +577,10 @@ pekko {
#jdk.virtualThreadScheduler.minRunnable
#jdk.unparker.maxPoolSize
fallback = "fork-join-executor"
+
+ # When using virtual-thread-executor, you can set the starting naumber
for virtual threads created by this dispatcher.
Review Comment:
The word 'naumber' is misspelled. It should be 'number'.
```suggestion
# When using virtual-thread-executor, you can set the starting
number for virtual threads created by this dispatcher.
```
##########
actor/src/main/resources/reference.conf:
##########
@@ -555,6 +559,10 @@ pekko {
# --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED
# --add-opens=java.base/java.lang=ALL-UNNAMED
virtualize = off
+
+ # When virtualize = on, you can set the starting naumber for virtual
threads created by this dispatcher.
Review Comment:
The word 'naumber' is misspelled. It should be 'number'.
##########
actor/src/main/scala/org/apache/pekko/dispatch/VirtualThreadSupport.scala:
##########
@@ -57,53 +57,57 @@ private[dispatch] object VirtualThreadSupport {
/**
* Create a virtual thread factory with the default Virtual Thread executor.
+ * @param prefix the prefix of the virtual thread name.
+ * @param start the starting number of the virtual thread name, if -1, the
number will not be appended.
*/
- def newVirtualThreadFactory(prefix: String): ThreadFactory = {
- require(isSupported, "Virtual thread is not supported.")
- try {
- val builderClass =
ClassLoader.getSystemClassLoader.loadClass("java.lang.Thread$Builder")
- val ofVirtualClass =
ClassLoader.getSystemClassLoader.loadClass("java.lang.Thread$Builder$OfVirtual")
- val ofVirtualMethod = lookup.findStatic(classOf[Thread], "ofVirtual",
MethodType.methodType(ofVirtualClass))
- var builder = ofVirtualMethod.invoke()
- val nameMethod = lookup.findVirtual(ofVirtualClass, "name",
- MethodType.methodType(ofVirtualClass, classOf[String], classOf[Long]))
- // TODO support replace scheduler when we drop Java 8 support
- val factoryMethod = lookup.findVirtual(builderClass, "factory",
MethodType.methodType(classOf[ThreadFactory]))
- builder = nameMethod.invoke(builder, prefix + "-virtual-thread-", zero)
- factoryMethod.invoke(builder).asInstanceOf[ThreadFactory]
- } catch {
- case NonFatal(e) =>
- // --add-opens java.base/java.lang=ALL-UNNAMED
- throw new UnsupportedOperationException("Failed to create virtual
thread factory", e)
- }
+ def newVirtualThreadFactory(prefix: String, start: Int): ThreadFactory = {
+ newVirtualThreadFactory(prefix, start, null)
}
/**
- * Create a virtual thread factory with the specified executor as the
scheduler of virtual thread.
+ * Create a virtual thread factory with the default Virtual Thread executor.
+ * @param prefix the prefix of the virtual thread name.
+ * @param start the starting number of the virtual thread name, if -1, the
number will not be appended.
+ * @param executor the executor to be used as the scheduler of virtual
thread. If null, the default scheduler will be used.
*/
- def newVirtualThreadFactory(prefix: String, executor: ExecutorService):
ThreadFactory =
+ def newVirtualThreadFactory(prefix: String, start: Int, executor:
ExecutorService): ThreadFactory = {
+ require(isSupported, "Virtual thread is not supported.")
+ require(prefix != null && prefix.nonEmpty, "prefix should not be null or
empty.")
try {
val builderClass =
ClassLoader.getSystemClassLoader.loadClass("java.lang.Thread$Builder")
val ofVirtualClass =
ClassLoader.getSystemClassLoader.loadClass("java.lang.Thread$Builder$OfVirtual")
- val ofVirtualMethod = classOf[Thread].getDeclaredMethod("ofVirtual")
- var builder = ofVirtualMethod.invoke(null)
+ val ofVirtualMethod = lookup.findStatic(classOf[Thread], "ofVirtual",
MethodType.methodType(ofVirtualClass))
+ var builder = ofVirtualMethod.invoke()
+ // set the name
+ if (start <= -1) {
+ val nameMethod = lookup.findVirtual(ofVirtualClass, "name",
+ MethodType.methodType(ofVirtualClass, classOf[String]))
+ builder = nameMethod.invoke(builder, prefix + "-virtual-thread")
+ } else {
+ val nameMethod = lookup.findVirtual(ofVirtualClass, "name",
+ MethodType.methodType(ofVirtualClass, classOf[String],
classOf[Long]))
+ builder = nameMethod.invoke(builder, prefix + "-virtual-thread-", zero)
Review Comment:
The `start` parameter is ignored when `start > -1`. The code should use the
actual `start` value instead of hardcoded `zero`. It should be
`java.lang.Long.valueOf(start.toLong)`.
```suggestion
builder = nameMethod.invoke(builder, prefix + "-virtual-thread-",
java.lang.Long.valueOf(start.toLong))
```
##########
actor/src/main/scala/org/apache/pekko/dispatch/VirtualThreadSupport.scala:
##########
@@ -57,53 +57,57 @@ private[dispatch] object VirtualThreadSupport {
/**
* Create a virtual thread factory with the default Virtual Thread executor.
+ * @param prefix the prefix of the virtual thread name.
+ * @param start the starting number of the virtual thread name, if -1, the
number will not be appended.
*/
- def newVirtualThreadFactory(prefix: String): ThreadFactory = {
- require(isSupported, "Virtual thread is not supported.")
- try {
- val builderClass =
ClassLoader.getSystemClassLoader.loadClass("java.lang.Thread$Builder")
- val ofVirtualClass =
ClassLoader.getSystemClassLoader.loadClass("java.lang.Thread$Builder$OfVirtual")
- val ofVirtualMethod = lookup.findStatic(classOf[Thread], "ofVirtual",
MethodType.methodType(ofVirtualClass))
- var builder = ofVirtualMethod.invoke()
- val nameMethod = lookup.findVirtual(ofVirtualClass, "name",
- MethodType.methodType(ofVirtualClass, classOf[String], classOf[Long]))
- // TODO support replace scheduler when we drop Java 8 support
- val factoryMethod = lookup.findVirtual(builderClass, "factory",
MethodType.methodType(classOf[ThreadFactory]))
- builder = nameMethod.invoke(builder, prefix + "-virtual-thread-", zero)
- factoryMethod.invoke(builder).asInstanceOf[ThreadFactory]
- } catch {
- case NonFatal(e) =>
- // --add-opens java.base/java.lang=ALL-UNNAMED
- throw new UnsupportedOperationException("Failed to create virtual
thread factory", e)
- }
+ def newVirtualThreadFactory(prefix: String, start: Int): ThreadFactory = {
+ newVirtualThreadFactory(prefix, start, null)
}
/**
- * Create a virtual thread factory with the specified executor as the
scheduler of virtual thread.
+ * Create a virtual thread factory with the default Virtual Thread executor.
+ * @param prefix the prefix of the virtual thread name.
+ * @param start the starting number of the virtual thread name, if -1, the
number will not be appended.
+ * @param executor the executor to be used as the scheduler of virtual
thread. If null, the default scheduler will be used.
*/
- def newVirtualThreadFactory(prefix: String, executor: ExecutorService):
ThreadFactory =
+ def newVirtualThreadFactory(prefix: String, start: Int, executor:
ExecutorService): ThreadFactory = {
+ require(isSupported, "Virtual thread is not supported.")
+ require(prefix != null && prefix.nonEmpty, "prefix should not be null or
empty.")
try {
val builderClass =
ClassLoader.getSystemClassLoader.loadClass("java.lang.Thread$Builder")
val ofVirtualClass =
ClassLoader.getSystemClassLoader.loadClass("java.lang.Thread$Builder$OfVirtual")
- val ofVirtualMethod = classOf[Thread].getDeclaredMethod("ofVirtual")
- var builder = ofVirtualMethod.invoke(null)
+ val ofVirtualMethod = lookup.findStatic(classOf[Thread], "ofVirtual",
MethodType.methodType(ofVirtualClass))
+ var builder = ofVirtualMethod.invoke()
+ // set the name
+ if (start <= -1) {
Review Comment:
The condition `start <= -1` should be `start < 0` or `start == -1`. Using
`<= -1` includes values like -2, -3, etc., which may not be intended behavior
according to the documentation that specifically mentions -1.
```suggestion
if (start == -1) {
```
##########
actor/src/main/resources/reference.conf:
##########
@@ -623,6 +635,10 @@ pekko {
# --add-opens=java.base/jdk.internal.misc=ALL-UNNAMED
# --add-opens=java.base/java.lang=ALL-UNNAMED
virtualize = off
+
+ # When virtualize = on, you can set the starting naumber for virtual
threads created by this dispatcher.
Review Comment:
The word 'naumber' is misspelled. It should be 'number'.
```suggestion
# When virtualize = on, you can set the starting number for virtual
threads created by this dispatcher.
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscr...@pekko.apache.org
For additional commands, e-mail: notifications-h...@pekko.apache.org
