perf: change Expr from trait to abstract class#786
Draft
He-Pin wants to merge 1 commit intodatabricks:masterfrom
Draft
perf: change Expr from trait to abstract class#786He-Pin wants to merge 1 commit intodatabricks:masterfrom
He-Pin wants to merge 1 commit intodatabricks:masterfrom
Conversation
Contributor
Author
|
In Netty, the ByteBuf is an abstract class too. |
2c7c1ac to
a51477b
Compare
Contributor
Author
|
Need update the readme too |
Motivation: trait methods use invokeinterface (~5-8ns, itable search) while abstract class methods use invokevirtual (~2-3ns, vtable). The e.tag dispatch in the evaluator hot path benefits from this. Modification: - Expr: trait → abstract class - Val: extends Expr directly (Val already extends Eval) - Literal: remove redundant 'with Expr' (inherited via Val) - Func: remove redundant 'with Expr' (inherited via Val) - TailCall: change def pos to var pos (required by abstract class Expr) - StaticOptimizer: simplify 'Val with Expr' casts to 'Val' Result: e.tag calls use invokevirtual instead of invokeinterface. All tests pass.
a51477b to
dcd1702
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
perf: change Expr from trait to abstract class
Motivation
On the JVM, calling a method through an interface reference (
invokeinterface) requires searching the interface method table (itable). Calling through a class reference (invokevirtual) uses a fixed-offset vtable lookup — a single indexed memory access. Cost difference: ~2-5ns per call.In the evaluator hot path:
e.tag(NewEvaluator's tableswitch dispatch)e.pos(position tracking)e.exprErrorString(error reporting)These are called on
Expr-typed references millions of times per evaluation. This adds up to measurable CPU overhead.Additionally,
instanceofchecks during pattern matching (e.g.,case e: Val) are faster on class hierarchies than interface hierarchies, because the JIT can use Class Hierarchy Analysis (CHA) to generate more aggressive code.Key Design Decision
Convert Expr from trait to abstract class:
invokeinterface(O(n) itable search) toinvokevirtual(O(1) vtable)Val extends Expr(class hierarchy, single inheritance)Evalremains trait (necessary forVal extends Expr with Eval)TailCall.posbecomesvar(required by abstract class constructor)case e: Valnow checks class hierarchy (faster) instead of mixed class+interfaceWhy not alternatives?
Val extends Expr with Eval(single inheritance in JVM)TailstrictableExpr,CompSpec,ObjBodyare not on dispatch hot path and benefit less from class hierarchyModifications
Expr.scalatrait Expr→abstract class ExprVal.scalaVal extends Eval→Val extends Expr with EvalVal.scalaLiteral extends Val with Expr→Literal extends Val(redundant)Val.scalaFunc extends Val with Expr→Func extends Val(redundant)Val.scalaTailCall.pos:def→var(required by abstract class)Val.scalaTailCall.exprErrorString: addoverride(now overrides Expr's default)StaticOptimizer.scalaVal with Exprpatterns →Val(Val is always Expr now)Benchmark Results
JMH Benchmark Results (JDK 21.0.10, Zulu, aarch64,
-f 1 -wi 8 -i 8):Evaluator-only (no parse/materialize)
Full pipeline (parse + eval + materialize)
Other
✅ Consistent improvement across all JVM benchmarks, with oldEvaluator (instanceof-based dispatch) benefiting most (-6.7%).
Scala Native
Native shows no change (expected — Scala Native fully devirtualizes both, so trait vs class is irrelevant).
Analysis
Why does this help?
e.tag,e.pos,e.exprErrorStringcalls happen millions of times, so per-call savings accumulateWhy is improvement visible in oldEvaluator but smaller in newEvaluator?
match { case e: Val => ... }) which compiles toinstanceofchecks — benefits directly from CHA improvementNo regression on Scala Native: Native backend fully devirtualizes both trait and abstract class implementations, so the JVM vtable/itable distinction doesn't apply.
Safety
✅ No semantic changes — Expr behavior is identical, just faster dispatch
✅ All tests pass — Full test suite (./mill __.test) passes on JVM/Native/JS
✅ Backward compatible — No public API changes (Expr is internal)
References
Result
✅ Rebased to latest master and verified. Abstract class Expr provides consistent 2-11% speedup on JVM, particularly for instanceof-heavy code paths. No regressions on Native. Ready for merge.