From 923519f31d44095eeb662d2e26af689e66633716 Mon Sep 17 00:00:00 2001 From: Stiopa Koltsov Date: Mon, 7 Dec 2020 08:09:22 -0800 Subject: [PATCH] Starlark: optimize StarlarkInt.Big comparison to StarlarkInt.Int{32,64} Perform comparison without conversion of smaller integers to `BigInteger`. `StarlarkInt.compareTo` does not allocate now. For this benchmark: ``` def test(): x = 17 << 77 for i in range(10): print(i) for j in range(10000000): x > 1 test() ``` ``` A: n=27 mean=4.262 std=0.203 se=0.039 min=4.036 med=4.193 B: n=27 mean=4.113 std=0.172 se=0.033 min=3.859 med=4.049 B/A: 0.965 0.941..0.990 (95% conf) ``` Speed up is about 7% when comparing to an integer outside of `BigInteger` cached range (-16..16). Finally, `StarlarkInt.Big` to `StarlarkInt.Big` comparison performance seems to stay the same (within 95% confidence interval after 100 test iterations). Closes #12638. PiperOrigin-RevId: 346095040 --- .../net/starlark/java/eval/StarlarkInt.java | 33 ++++++++++--------- 1 file changed, 17 insertions(+), 16 deletions(-) diff --git a/src/main/java/net/starlark/java/eval/StarlarkInt.java b/src/main/java/net/starlark/java/eval/StarlarkInt.java index d63186ba92cf29..d810c6b2e15770 100644 --- a/src/main/java/net/starlark/java/eval/StarlarkInt.java +++ b/src/main/java/net/starlark/java/eval/StarlarkInt.java @@ -427,26 +427,27 @@ public int compareTo(StarlarkInt x) { // binary operators - // In the common case, both operands are int32, so the operations - // can be done using longs with minimal fuss around overflows. - // All other combinations are promoted to BigInteger. - // TODO(adonovan): use long x long operations even if one or both - // operands are Int64; promote to Big x Big only upon overflow. - // (See the revision history for the necessary overflow checks.) - - /** Returns a value whose signum is equal to x - y. */ + /** Returns signum(x - y). */ public static int compare(StarlarkInt x, StarlarkInt y) { - if (x instanceof Int32 && y instanceof Int32) { - return Integer.compare(((Int32) x).v, ((Int32) y).v); - } - + // If both arguments are big, we compare BigIntegers. + // If neither argument is big, we compare longs. + // If only one argument is big, its magnitude is greater + // than the other operand, so only its sign matters. + // + // We avoid unnecessary branches. try { - return Long.compare(x.toLongFast(), y.toLongFast()); + long xl = x.toLongFast(); + try { + long yl = y.toLongFast(); + return Long.compare(xl, yl); // (long, long) + } catch (Overflow unused) { + return -((Big) y).v.signum(); // (long, big) + } } catch (Overflow unused) { - /* fall through */ + return y instanceof Big + ? ((Big) x).v.compareTo(((Big) y).v) // (big, big) + : ((Big) x).v.signum(); // (big, long) } - - return x.toBigInteger().compareTo(y.toBigInteger()); } /** Returns x + y. */