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]