sunchao commented on code in PR #5415:
URL: https://github.com/apache/datafusion-comet/pull/5415#discussion_r3839045154
##########
spark/src/test/scala/org/apache/spark/sql/benchmark/CometRegExpBenchmark.scala:
##########
@@ -50,73 +51,91 @@ case class RegExpPattern(name: String, pattern: String)
*/
object CometRegExpBenchmark extends CometBenchmarkBase {
- // Patterns chosen to span common rlike shapes. Avoid Java-only constructs
- // that the native (Rust) path cannot accept, since those would be skipped
- // rather than benchmarked in the native case.
- private val patterns = List(
+ // Analyzer-admitted patterns. Default Comet exec is already native, so do
not add a
+ // "JVM regex" case: it would silently measure the native path.
+ private val inSubsetPatterns = List(
RegExpPattern("character_class", "[0-9]+"),
- RegExpPattern("anchored", "^[0-9]"),
RegExpPattern("alternation", "abc|def|ghi"),
RegExpPattern("multi_class", "[a-zA-Z][0-9]+"),
RegExpPattern("repetition", "(ab){2,}"))
+ // Analyzer-rejected, Rust-accepted. Default exec is the JVM dispatcher;
opt-in is native.
+ // Input data is ASCII (REPEAT of numeric strings) so `\d` vs `[0-9]` does
not change hits.
+ private val outOfSubsetPatterns =
List(RegExpPattern("digit_class_shorthand", "\\d+"))
+
override def runCometBenchmark(mainArgs: Array[String]): Unit = {
- runBenchmarkWithTable("rlike modes", 1024) { v =>
+ runBenchmarkWithTable("rlike modes", 1024 * 1024) { v =>
withTempPath { dir =>
withTempTable("parquetV1Table") {
prepareTable(
dir,
spark.sql(s"SELECT REPEAT(CAST(value AS STRING), 10) AS c1 FROM
$tbl"))
- patterns.foreach { p =>
+ inSubsetPatterns.foreach { p =>
+ val query = s"select c1 rlike '${p.pattern}' from parquetV1Table"
+ runBenchmark(p.name) {
+ runInSubsetModes(p.name, v, query)
+ }
+ }
+ outOfSubsetPatterns.foreach { p =>
val query = s"select c1 rlike '${p.pattern}' from parquetV1Table"
Review Comment:
[P2] Preserve the backslash in the out-of-subset benchmark SQL
With the default `spark.sql.parser.escapedStringLiterals=false`,
interpolating the new `\d+` pattern here makes Spark parse it as `d+`
(reproduced with Spark 3.5.9 and 4.0.4). `CometRegex` admits `d+`, so the case
labelled `Comet (Exec, JVM regex)` selects the automatic native branch just
like the opted-in case. The generated numeric strings also match none of the
rows instead of every row. This leaves the new four-way comparison without a
JVM-dispatcher measurement. Please preserve the backslash with an escaped or
raw SQL literal, as the parity suite already does, before passing this query to
`runOutOfSubsetModes`.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
