perf: validate actor path elements without String.charAt

actor-tests/src/test/scala/org/apache/pekko/actor/ActorPathSpec.scala

@@ -16,9 +16,10 @@ package org.apache.pekko.actor
 import java.net.MalformedURLException
 
 import org.scalatest.matchers.should.Matchers
+import org.scalatest.prop.TableDrivenPropertyChecks
 import org.scalatest.wordspec.AnyWordSpec
 
-class ActorPathSpec extends AnyWordSpec with Matchers {
+class ActorPathSpec extends AnyWordSpec with Matchers with TableDrivenPropertyChecks {
 
   "An ActorPath" must {
 
@@ -81,6 +82,57 @@ class ActorPathSpec extends AnyWordSpec with Matchers {
         "must not be empty")
     }
 
+    "accept valid path elements" in {
+      val valid = Table(
+        "name",
+        "a",
+        "actor-1",
+        "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789",
+        "-_.*$+:@&=,!~';",
+        "a$" + "b",
+        "%20",
+        "%ff%FF%0a",
+        "a%20b",
+        "x" * 10000)
+      forAll(valid) { name =>
+        ActorPath.isValidPathElement(name) should ===(true)
+        ActorPath.validatePathElement(name)
+      }
+    }
+
+    "reject invalid path elements and report the position" in {
+      def positionOf(name: String): Int =
+        intercept[InvalidActorNameException](ActorPath.validatePathElement(name)).getMessage match {
+          case msg if msg.contains("at position: ") =>
+            msg.split("at position: ")(1).takeWhile(_.isDigit).toInt
+          case msg => fail(s"unexpected message [$msg]")
+        }
+
+      val invalid = Table(
+        ("name", "position"),
+        ("$" + "a", 0), // `$` is reserved for system names at the start
+        ("a b", 1),
+        ("a/b", 1),
+        ("a#b", 1),
+        ("a?b", 1),
+        ("a%", 1), // `%` is only valid as `%XX`
+        ("a%2", 1),
+        ("a%2g", 1),
+        ("a%g2", 1),
+        ("caf\u00e9", 3), // Latin-1 but not ASCII
+        ("na\u00efve-actor", 2),
+        ("\u4e2d\u6587", 0), // outside Latin-1
+        ("actor-\u4e2d", 6),
+        ("actor-\ud83d\ude00", 6), // surrogate pair
+        ("a\u0000b", 1),
+        ("a\u007fb", 1),
+        ("x" * 100 + "\u00e9", 100))
+      forAll(invalid) { (name, position) =>
+        ActorPath.isValidPathElement(name) should ===(false)
+        positionOf(name) should ===(position)
+      }
+    }
+
     "create correct toStringWithAddress" in {
       val local = Address("pekko", "mysys")
       val a = local.copy(host = Some("aaa"), port = Some(7355))

actor/src/main/scala/org/apache/pekko/actor/ActorPath.scala

@@ -14,6 +14,7 @@
 package org.apache.pekko.actor
 import java.lang.{ StringBuilder => JStringBuilder }
 import java.net.MalformedURLException
+import java.nio.charset.StandardCharsets
 
 import scala.annotation.nowarn
 import scala.annotation.tailrec
@@ -79,6 +80,26 @@ object ActorPath {
   private final val ValidPathCode = -1
   private final val EmptyPathCode = -2
 
+  private final val ValidCharFlag: Byte = 1
+  private final val HexCharFlag: Byte = 2
+
+  /**
+   * Lookup table over the 7-bit ASCII range: bit 0 marks a character allowed in a path element,
+   * bit 1 marks a hex digit (for `%XX` escapes). Everything at or above 0x80 is invalid.
+   */
+  private final val PathCharFlags: Array[Byte] = {
+    val flags = new Array[Byte](128)
+    def set(c: Char, flag: Byte): Unit = flags(c) = (flags(c) | flag).toByte
+    ('a' to 'z').foreach(set(_, ValidCharFlag))
+    ('A' to 'Z').foreach(set(_, ValidCharFlag))
+    ('0' to '9').foreach(set(_, ValidCharFlag))
+    ValidSymbols.foreach(set(_, ValidCharFlag))
+    ('a' to 'f').foreach(set(_, HexCharFlag))
+    ('A' to 'F').foreach(set(_, HexCharFlag))
+    ('0' to '9').foreach(set(_, HexCharFlag))
+    flags
+  }
+
   /**
    * Validates the given actor path element and throws an [[InvalidActorNameException]] if invalid.
    * See [[#isValidPathElement]] for a non-throwing version.
@@ -127,29 +148,29 @@ object ActorPath {
   private final def findInvalidPathElementCharPosition(s: String): Int =
     if (s.isEmpty) EmptyPathCode
     else {
-      def isValidChar(c: Char): Boolean =
-        (c >= 'a' && c <= 'z') ||
-        (c >= 'A' && c <= 'Z') ||
-        (c >= '0' && c <= '9') ||
-        (ValidSymbols.indexOf(c) != -1)
-
-      def isHexChar(c: Char): Boolean =
-        (c >= 'a' && c <= 'f') ||
-        (c >= 'A' && c <= 'F') ||
-        (c >= '0' && c <= '9')
-
-      val len = s.length
-      def validate(pos: Int): Int =
-        if (pos < len)
-          s.charAt(pos) match {
-            case c if isValidChar(c)                                                                  => validate(pos + 1)
-            case '%' if pos + 2 < len && isHexChar(s.charAt(pos + 1)) && isHexChar(s.charAt(pos + 2)) =>
-              validate(pos + 3)
-            case _ => pos
-          }
-        else ValidPathCode
-
-      if (len > 0 && s.charAt(0) != '$') validate(0) else 0
+      // Copy the string out once (an intrinsic array copy for Latin-1 strings) and scan the byte
+      // array with a table lookup. This avoids `String.charAt` in the loop, whose Latin-1/UTF-16
+      // coder branch shares one JVM-wide profile and is easily polluted by unrelated non-ASCII
+      // strings. Characters outside Latin-1 are encoded as '?', which is invalid, and characters
+      // in 0x80-0xFF are negative bytes, which are also invalid, so positions are preserved.
+      val bytes = s.getBytes(StandardCharsets.ISO_8859_1)
+      val len = bytes.length
+      val flags = PathCharFlags
+
+      def flagsAt(pos: Int): Int = {
+        val b = bytes(pos)
+        if (b >= 0) flags(b) else 0
+      }
+
+      @tailrec def validate(pos: Int): Int =
+        if (pos >= len) ValidPathCode
+        else if ((flagsAt(pos) & ValidCharFlag) != 0) validate(pos + 1)
+        else if (bytes(pos) == '%' && pos + 2 < len &&
+          (flagsAt(pos + 1) & HexCharFlag) != 0 && (flagsAt(pos + 2) & HexCharFlag) != 0)
+          validate(pos + 3)
+        else pos
+
+      if (bytes(0) != '$') validate(0) else 0
     }
 
   private[pekko] final val emptyActorPath: immutable.Iterable[String] = List("")

bench-jmh/src/main/scala/org/apache/pekko/actor/ActorPathValidationBenchmark.scala

@@ -22,6 +22,8 @@ import org.openjdk.jmh.annotations.Fork
 import org.openjdk.jmh.annotations.Measurement
 import org.openjdk.jmh.annotations.Mode
 import org.openjdk.jmh.annotations.OutputTimeUnit
+import org.openjdk.jmh.annotations.Param
+import org.openjdk.jmh.annotations.Setup
 import org.openjdk.jmh.annotations.State
 import org.openjdk.jmh.annotations.Warmup
 
@@ -32,6 +34,21 @@ import org.openjdk.jmh.annotations.Warmup
 
 [info] a.a.ActorPathValidationBenchmark.handLoopActor_1    thrpt        20       38.825        3.378   ops/us
 [info] a.a.ActorPathValidationBenchmark.oldActor_1         thrpt        20        1.585        0.090   ops/us
+
+`polluted = true` first exercises String.charAt with UTF-16 strings so its Latin-1/UTF-16 coder branch
+profile (collected once, JVM-wide, in String.charAt's own bytecode) sees both coders, as it would in
+any process that ever handles non-ASCII text. charAt-based scanners then get both coders compiled into
+their loop; the byte-table scanner (isValidPathElement) is unaffected:
+
+[info] Benchmark                                                  (polluted)   Mode  Cnt    Score    Error   Units
+[info] ActorPathValidationBenchmark.charAtLoop7000                     false  thrpt    5    0.197 ±  0.028  ops/us
+[info] ActorPathValidationBenchmark.charAtLoop7000                      true  thrpt    5    0.107 ±  0.017  ops/us
+[info] ActorPathValidationBenchmark.charAtLoopActor_1                  false  thrpt    5  100.954 ± 14.868  ops/us
+[info] ActorPathValidationBenchmark.charAtLoopActor_1                   true  thrpt    5   49.306 ±  8.706  ops/us
+[info] ActorPathValidationBenchmark.handLoop7000                       false  thrpt    5    0.176 ±  0.012  ops/us
+[info] ActorPathValidationBenchmark.handLoop7000                        true  thrpt    5    0.142 ±  0.094  ops/us
+[info] ActorPathValidationBenchmark.handLoopActor_1                    false  thrpt    5  138.841 ± 111.054 ops/us
+[info] ActorPathValidationBenchmark.handLoopActor_1                     true  thrpt    5  162.126 ± 281.298 ops/us
  */
 @Fork(2)
 @State(JmhScope.Benchmark)
@@ -41,21 +58,75 @@ import org.openjdk.jmh.annotations.Warmup
 @OutputTimeUnit(TimeUnit.MICROSECONDS)
 class ActorPathValidationBenchmark {
 
+  @Param(Array("false", "true"))
+  var polluted: Boolean = false
+
   final val a = "actor-1"
   final val s = "687474703a2f2f74686566727569742e636f6d2f26683d37617165716378357926656e" * 100
 
   final val ElementRegex = """(?:[-\w:@&=+,.!~*'_;]|%\p{XDigit}{2})(?:[-\w:@&=+,.!~*'$_;]|%\p{XDigit}{2})*""".r
 
+  @Setup
+  def setup(): Unit = {
+    if (polluted) {
+      val utf16 = "årsrapport-ünïcödé-中文-é"
+      var acc = 0
+      var i = 0
+      while (i < 2000000) {
+        acc += (if (charAtLoop(utf16 + i)) 1 else 0) + utf16.charAt(i % utf16.length)
+        i += 1
+      }
+      if (acc == 42) println("unlikely")
+    }
+  }
+
   //  @Benchmark // blows up with stack overflow, we know
   def old7000: Option[List[String]] = ElementRegex.unapplySeq(s)
 
   @Benchmark
   def handLoop7000: Boolean = ActorPath.isValidPathElement(s)
 
+  @Benchmark
+  def charAtLoop7000: Boolean = charAtLoop(s)
+
   @Benchmark
   def oldActor_1: Option[List[String]] = ElementRegex.unapplySeq(a)
 
   @Benchmark
   def handLoopActor_1: Boolean = ActorPath.isValidPathElement(a)
 
+  @Benchmark
+  def charAtLoopActor_1: Boolean = charAtLoop(a)
+
+  // the String.charAt based validator that isValidPathElement used before the byte-table version
+  private final val ValidSymbols = """-_.*$+:@&=,!~';"""
+
+  private def charAtLoop(s: String): Boolean =
+    if (s.isEmpty) false
+    else {
+      def isValidChar(c: Char): Boolean =
+        (c >= 'a' && c <= 'z') ||
+        (c >= 'A' && c <= 'Z') ||
+        (c >= '0' && c <= '9') ||
+        (ValidSymbols.indexOf(c) != -1)
+
+      def isHexChar(c: Char): Boolean =
+        (c >= 'a' && c <= 'f') ||
+        (c >= 'A' && c <= 'F') ||
+        (c >= '0' && c <= '9')
+
+      val len = s.length
+      def validate(pos: Int): Int =
+        if (pos < len)
+          s.charAt(pos) match {
+            case c if isValidChar(c)                                                                  => validate(pos + 1)
+            case '%' if pos + 2 < len && isHexChar(s.charAt(pos + 1)) && isHexChar(s.charAt(pos + 2)) =>
+              validate(pos + 3)
+            case _ => pos
+          }
+        else -1
+
+      (if (len > 0 && s.charAt(0) != '$') validate(0) else 0) == -1
+    }
+
 }