From be91cad8f68089244aa9f92e5f2d5f648dc28842 Mon Sep 17 00:00:00 2001 From: Gengliang Wang Date: Sat, 30 May 2026 23:38:18 +0000 Subject: [PATCH] [SPARK-57178][SQL] Simplify Asinh codegen by extracting a static Java helper ### What changes were proposed in this pull request? Add `ExpressionImplUtils.asinh(double x)` (the fdlibm `s_asinh.c` algorithm) and route `Asinh`'s eval and codegen paths through it. `Asinh.doGenCode` previously emitted a ~14-line inline five-branch if/else; it now emits a single `ExpressionImplUtils.asinh(...)` call via `defineCodeGen`, and the eval path calls the same helper instead of an inline lambda copy of the algorithm. `asinh` returns a primitive `double`, so the helper is a clean drop-in for both paths with no boxing and no null handling. Sibling of SPARK-57177 (Acosh). ### Why are the changes needed? Part of SPARK-56908 (umbrella). Collapsing the duplicated ~14-line algorithm to a single call shrinks the generated Java for every stage that uses `asinh`, and removes the eval/codegen duplication of the same fdlibm algorithm. ### Does this PR introduce _any_ user-facing change? No. The compiled behavior is identical; only the emitted Java source text changes. ### How was this patch tested? ``` build/sbt "catalyst/testOnly *MathExpressionsSuite" ``` Pass, including `asinh` and `SPARK-56089: asinh/acosh fdlibm algorithm coverage` (exercised both with and without whole-stage codegen). ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Opus 4.8) Co-authored-by: Isaac --- .../expressions/ExpressionImplUtils.java | 24 ++++++++++ .../expressions/mathExpressions.scala | 44 ++----------------- 2 files changed, 28 insertions(+), 40 deletions(-) diff --git a/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionImplUtils.java b/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionImplUtils.java index a5228edc33c83..a7d98f2fbab2a 100644 --- a/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionImplUtils.java +++ b/sql/catalyst/src/main/java/org/apache/spark/sql/catalyst/expressions/ExpressionImplUtils.java @@ -342,4 +342,28 @@ public static UTF8String quote(UTF8String str) { String sp = str.toString().replaceAll(qtChar, qtCharRep); return UTF8String.fromString(qtChar + sp + qtChar); } + + /** + * Inverse hyperbolic sine for the {@code asinh} expression, using the fdlibm + * {@code s_asinh.c} algorithm (odd function, so the sign of {@code x} is + * preserved). Shared by the eval and codegen paths so the generated Java is a + * single call rather than an inline five-branch if/else. + */ + public static double asinh(double x) { + double ax = Math.abs(x); + double w; + if (Double.isInfinite(ax) || Double.isNaN(ax)) { + w = ax; + } else if (ax < 1.0 / (1 << 28)) { + w = ax; + } else if (ax > (1 << 28)) { + w = StrictMath.log(ax) + StrictMath.log(2.0); + } else if (ax > 2.0) { + w = StrictMath.log(2.0 * ax + 1.0 / (Math.sqrt(x * x + 1.0) + ax)); + } else { + double t = x * x; + w = StrictMath.log1p(ax + t / (1.0 + Math.sqrt(1.0 + t))); + } + return Math.copySign(w, x); + } } diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala index 56fbb00131d57..a83ca5295cc0d 100644 --- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala +++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/mathExpressions.scala @@ -877,47 +877,11 @@ case class Sinh(child: Expression) extends UnaryMathExpression(math.sinh, "SINH" since = "3.0.0", group = "math_funcs") case class Asinh(child: Expression) - extends UnaryMathExpression((x: Double) => { - // fdlibm s_asinh.c algorithm - val ax = Math.abs(x) - val w = if (ax.isInfinite || ax.isNaN) { - ax - } else if (ax < 1.0 / (1 << 28)) { - ax - } else if (ax > (1 << 28)) { - StrictMath.log(ax) + StrictMath.log(2.0) - } else if (ax > 2.0) { - StrictMath.log(2.0 * ax + 1.0 / (math.sqrt(x * x + 1.0) + ax)) - } else { - val t = x * x - StrictMath.log1p(ax + t / (1.0 + math.sqrt(1.0 + t))) - } - Math.copySign(w, x) - }, "ASINH") { + // fdlibm s_asinh.c algorithm, shared with codegen via ExpressionImplUtils.asinh. + extends UnaryMathExpression((x: Double) => ExpressionImplUtils.asinh(x), "ASINH") { override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = { - nullSafeCodeGen(ctx, ev, c => { - val sm = "java.lang.StrictMath" - val ax = ctx.freshName("ax") - val w = ctx.freshName("w") - val t = ctx.freshName("t") - s""" - |double $ax = java.lang.Math.abs($c); - |double $w; - |if (java.lang.Double.isInfinite($ax) || java.lang.Double.isNaN($ax)) { - | $w = $ax; - |} else if ($ax < ${1.0 / (1 << 28)}) { - | $w = $ax; - |} else if ($ax > ${1 << 28}.0) { - | $w = $sm.log($ax) + $sm.log(2.0); - |} else if ($ax > 2.0) { - | $w = $sm.log(2.0 * $ax + 1.0 / (java.lang.Math.sqrt($c * $c + 1.0) + $ax)); - |} else { - | double $t = $c * $c; - | $w = $sm.log1p($ax + $t / (1.0 + java.lang.Math.sqrt(1.0 + $t))); - |} - |${ev.value} = java.lang.Math.copySign($w, $c); - |""".stripMargin - }) + val utils = classOf[ExpressionImplUtils].getName + defineCodeGen(ctx, ev, c => s"$utils.asinh($c)") } override protected def withNewChildInternal(newChild: Expression): Asinh = copy(child = newChild) }