This is an automated email from the ASF dual-hosted git repository.

He-Pin pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/pekko.git


The following commit(s) were added to refs/heads/main by this push:
     new e336453c66 perf: validate actor path elements without String.charAt 
(#3542)
e336453c66 is described below

commit e336453c66e730b55ed990cd920bd218650a7fdd
Author: PJ Fanning <[email protected]>
AuthorDate: Fri Oct 9 08:37:54 2026 +0100

    perf: validate actor path elements without String.charAt (#3542)
    
    Motivation:
    `ActorPath.findInvalidPathElementCharPosition` runs for every named 
`actorOf`. It scanned the name with `String.charAt` inside a pattern match with 
guards, plus a `ValidSymbols.indexOf` for every non-alphanumeric character.
    
    `String.charAt` inlines to `isLatin1() ? StringLatin1.charAt : 
StringUTF16.charAt`, and that branch is profiled once, JVM-wide, in 
`String.charAt`'s own bytecode. Any non-ASCII string handled anywhere in the 
process (a Jackson payload, a log line) pollutes that profile, after which C2 
compiles both coders into every `charAt` loop. Measured locally, the validator 
halves in throughput once the profile is polluted.
    
    Modification:
    - Copy the element out once with `getBytes(ISO_8859_1)` — an intrinsic 
array copy for Latin-1 strings — and scan the `byte[]` with a 128-entry flag 
table (valid-char bit, hex-digit bit) instead of calling `charAt` per 
character. Characters outside Latin-1 encode as `?` and 0x80–0xFF as negative 
bytes; both are invalid, so the accepted set and the reported position are 
unchanged.
    - Add directional `ActorPathSpec` tests for accepted names, rejected names 
and the reported error position, including Latin-1, non-Latin-1 and 
surrogate-pair input.
    - Add an exhaustive sweep over all 65,536 chars asserting 
`isValidPathElement` agrees with an independently defined valid-char/hex-digit 
predicate (at position 0, after another char, and inside a `%XX` escape).
    - Extend `ActorPathValidationBenchmark` with a `polluted` param that 
pre-warms `String.charAt` with UTF-16 strings, and keep the previous 
`charAt`-based validator there as `charAtLoop*` for comparison.
    
    Result:
    Name validation no longer depends on the `String.charAt` coder profile. 
Local JMH (`-f 3 -wi 5 -w 2s -i 10 -r 2s`):
    
    | Benchmark | polluted=false | polluted=true |
    |---|---|---|
    | `charAtLoopActor_1` (previous code) | 108.3 ± 4.3 ops/µs | 41.8 ± 0.5 
ops/µs |
    | `handLoopActor_1` (this PR) | 161.0 ± 6.8 ops/µs | 162.0 ± 3.5 ops/µs |
    | `charAtLoop7000` (previous code) | 0.152 ± 0.003 ops/µs | 0.086 ± 0.004 
ops/µs |
    | `handLoop7000` (this PR) | 0.101 ± 0.014 ops/µs | 0.096 ± 0.007 ops/µs |
    
    For typical short names this PR is ~1.5× faster unpolluted and ~3.9× faster 
polluted. For a pathological 7000-char name it is ~34% slower than an 
unpolluted `charAt` loop (the `getBytes` copy dominates) and roughly on par 
when polluted.
    
    I tried the same treatment on the other hot `charAt` users, 
`MurmurHash.stringHash` and `Helpers.base64`, and measured no benefit (hash 
arithmetic and `StringBuilder.append` dominate; `toCharArray` only adds an 
allocation), so those are left as they are.
    
    Tests:
    - `sbt "actor-tests/testOnly org.apache.pekko.actor.ActorPathSpec 
org.apache.pekko.actor.LocalActorRefProviderSpec"` — 25 passed
    - `sbt "actor-tests/testOnly org.apache.pekko.actor.ActorPathSpec"` after 
adding the exhaustive sweep — 18 passed (sweep takes under a second)
    - `sbt "actor-tests/testOnly 
org.apache.pekko.routing.ConsistentHashingRouterSpec"` — passed
    - `sbt "actor/mimaReportBinaryIssues"` — no issues
    - `sbt "bench-jmh/Jmh/compile"` and `sbt "bench-jmh/Jmh/run -f 3 -wi 5 -w 
2s -i 10 -r 2s .*ActorPathValidationBenchmark.(handLoop|charAtLoop).*"`
    - `scalafmt` on changed Scala files, `git diff --check`
    
    References:
    None - performance follow-up to the hand-written actor name validator
---
 .../org/apache/pekko/actor/ActorPathSpec.scala     | 80 +++++++++++++++++++++-
 .../scala/org/apache/pekko/actor/ActorPath.scala   | 67 +++++++++++-------
 .../pekko/actor/ActorPathValidationBenchmark.scala | 73 ++++++++++++++++++++
 3 files changed, 196 insertions(+), 24 deletions(-)

diff --git 
a/actor-tests/src/test/scala/org/apache/pekko/actor/ActorPathSpec.scala 
b/actor-tests/src/test/scala/org/apache/pekko/actor/ActorPathSpec.scala
index a7573979fb..65b5951ac4 100644
--- a/actor-tests/src/test/scala/org/apache/pekko/actor/ActorPathSpec.scala
+++ b/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,83 @@ 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)
+      }
+    }
+
+    "classify every char like the reference predicate" in {
+      // Independent, exhaustive definition of the accepted alphabet, so the 
table-driven validator
+      // is locked to it for every one of the 65,536 chars (including 
0x80-0xFF and surrogates).
+      def isValidChar(c: Char): Boolean =
+        (c >= 'a' && c <= 'z') ||
+        (c >= 'A' && c <= 'Z') ||
+        (c >= '0' && c <= '9') ||
+        "-_.*$+:@&=,!~';".contains(c)
+      def isHexChar(c: Char): Boolean =
+        (c >= 'a' && c <= 'f') ||
+        (c >= 'A' && c <= 'F') ||
+        (c >= '0' && c <= '9')
+
+      var c = 0
+      while (c <= Char.MaxValue) {
+        val ch = c.toChar
+        withClue(f"char U+$c%04X: ") {
+          ActorPath.isValidPathElement(ch.toString) should ===(isValidChar(ch) 
&& ch != '$')
+          ActorPath.isValidPathElement("a" + ch) should ===(isValidChar(ch))
+          ActorPath.isValidPathElement("%" + ch + "0") should 
===(isHexChar(ch))
+          ActorPath.isValidPathElement("%0" + ch) should ===(isHexChar(ch))
+        }
+        c += 1
+      }
+    }
+
     "create correct toStringWithAddress" in {
       val local = Address("pekko", "mysys")
       val a = local.copy(host = Some("aaa"), port = Some(7355))
diff --git a/actor/src/main/scala/org/apache/pekko/actor/ActorPath.scala 
b/actor/src/main/scala/org/apache/pekko/actor/ActorPath.scala
index ef43038b54..2ecc4a5cd1 100644
--- a/actor/src/main/scala/org/apache/pekko/actor/ActorPath.scala
+++ b/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("")
diff --git 
a/bench-jmh/src/main/scala/org/apache/pekko/actor/ActorPathValidationBenchmark.scala
 
b/bench-jmh/src/main/scala/org/apache/pekko/actor/ActorPathValidationBenchmark.scala
index 780bbf096c..88444789c6 100644
--- 
a/bench-jmh/src/main/scala/org/apache/pekko/actor/ActorPathValidationBenchmark.scala
+++ 
b/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,23 @@ 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. For 
very long names the
+byte-table scanner pays for the getBytes copy and is slower than an unpolluted 
charAt loop
+(-f 3 -wi 5 -w 2s -i 10 -r 2s):
+
+[info] Benchmark                                       (polluted)   Mode  Cnt  
  Score   Error   Units
+[info] ActorPathValidationBenchmark.charAtLoop7000          false  thrpt   30  
  0.152 ± 0.003  ops/us
+[info] ActorPathValidationBenchmark.charAtLoop7000           true  thrpt   30  
  0.086 ± 0.004  ops/us
+[info] ActorPathValidationBenchmark.charAtLoopActor_1       false  thrpt   30  
108.294 ± 4.260  ops/us
+[info] ActorPathValidationBenchmark.charAtLoopActor_1        true  thrpt   30  
 41.834 ± 0.544  ops/us
+[info] ActorPathValidationBenchmark.handLoop7000            false  thrpt   30  
  0.101 ± 0.014  ops/us
+[info] ActorPathValidationBenchmark.handLoop7000             true  thrpt   30  
  0.096 ± 0.007  ops/us
+[info] ActorPathValidationBenchmark.handLoopActor_1         false  thrpt   30  
160.988 ± 6.836  ops/us
+[info] ActorPathValidationBenchmark.handLoopActor_1          true  thrpt   30  
161.953 ± 3.548  ops/us
  */
 @Fork(2)
 @State(JmhScope.Benchmark)
@@ -41,21 +60,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
+    }
+
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to