(Decimal_Spec) comparison failures on Oracle GraalVM

[JaroslavTulach]

There are intermittent failures (but more frequent recent days) when running test/Base_Tests on Oracle GraalVM.

[FAILED] (Decimal_Spec) comparison: [10/12, 1165ms]️ 
Error: Expected False to be True (at test/Base_Tests/src/Data/Decimal_Spec.enso:325:17-48).
     - [FAILED] should compare correctly to Integer and Float [1020ms]
️         Reason: Expected False to be True (at test/Base_Tests/src/Data/Decimal_Spec.enso:325:17-48).
️ 
️ 
️     - [FAILED] == mixed float/decimal comparisons should attach a warning [15ms]
Error: Expected the result to contain a warning: 
(Loss_Of_Numeric_Precision.Warning (original_value = 0.1) (new_value = 0.1)), 
but it did not. The warnings were [] (at test/Base_Tests/src/Data/Decimal_Spec.enso:395:17-106).
️
️ 
2772 tests succeeded.
2 tests failed.
81 tests skipped.
13 groups skipped.

It is (sometimes) possible to reproduce it locally (with 0840be0) I was able to do:

ENSO_LAUNCHER=native,test sbt --java-home ~/bin/graalvm-ee/
sbt:enso> buildEngineDistribution
sbt:enso> runEngineDistribution --run test/Base_Tests

2694 tests succeeded.
1 tests failed.
90 tests skipped.
21 groups skipped.

Failed tests: 'Decimal\.to_float.should.compare.correctly.with.the.original.Decimal'

however the reproducing steps aren't completely reliable. When running runEngineDistribution --run test/Base_Tests for second time, I get no failures.

[JaroslavTulach]

The failures

[FAILED] (Decimal_Spec) comparison: [10/12, 883ms]                    
    - [FAILED] should compare correctly to Integer and Float [788ms]
        Reason: -1.7976931348623157E308 did not equal -1.7976931348623157E+308
        (at test/Base_Tests/src/Data/Decimal_Spec.enso:324:17-41).

Reproducer

Those failures can be simulated by building the native image locally and then executing the base tests:

enso$ ENSO_LAUNCHER=native,debug,test sbt --java-home ~/bin/graalvm-ee/
enso$ ./enso --run test/Base_Tests

seems to trigger some of the listed failures relatively frequently.

There is DecimalSpec14996 branch that contains the latest changes useful for the reproducer.

[JaroslavTulach]

Workaround

While trying to analyze the failures I probably found a workaround. At least I am not able to reproduce the failure (yet) with these changes. Applying this workaround may mitigate the CI failures anytime. I'll however continue my investigations...

diff --git test/Base_Tests/src/Data/Decimal_Spec.enso test/Base_Tests/src/Data/Decimal_Spec.enso
index 0adb3464a5..979077e88d 100644
--- test/Base_Tests/src/Data/Decimal_Spec.enso
+++ test/Base_Tests/src/Data/Decimal_Spec.enso
@@ -15,6 +15,24 @@ Decimal.should_have_rep self rep = self.internal_representation . should_equal r
 
 id_decimal (x : Decimal) -> Decimal = x
 
+private assert_eq_is_true ~a ~b count=10 =
+    res = a == b
+    if res.not then
+        IO.print_err "Unexpectedly not equal! Press Enter to try again."
+        if count <= 0 then
+            Test.fail "Not equal:"+a.to_text+" but was"+b.to_text+" yet the result was: "+res.to_text
+        IO.print_err "Got:"+IO.readln
+        assert_eq_is_true a... b... count-1
+
+private assert_not_eq_is_false ~a ~b count=10 =
+    res = a != b
+    if res then
+        IO.print_err "Unexpectedly not equal! Press Enter to try again."
+        if count <= 0 then
+            Test.fail "Not equal:"+a.to_text+" but was"+b.to_text+" yet the result was: "+res.to_text
+        IO.print_err "Got:"+IO.readln
+        assert_not_eq_is_false a... b... count-1
+
 add_specs suite_builder =
     suite_builder.group "(Decimal_Spec) construction" group_builder->
         group_builder.specify "should be able to construct a Decimal with dec" <|
@@ -320,12 +338,12 @@ add_specs suite_builder =
                 d = Decimal.new v . remove_warnings
                 expected = pr.at 1
 
-                d . should_equal expected
-                expected . should_equal d
-                (d == expected) . should_be_true
-                (expected == d) . should_be_true
-                (d != expected) . should_be_false
-                (expected != d) . should_be_false
+                assert_eq_is_true d expected
+                assert_eq_is_true expected d
+                assert_eq_is_true (d == expected) True
+                assert_eq_is_true (expected == d) True
+                assert_not_eq_is_false d expected
+                assert_not_eq_is_false expected d
 
                 if v.is_a Text . not then
                     lesser = if v > 10000 then v/2 else if v < -10000 then v*2 else v-1
@@ -1033,7 +1051,7 @@ add_specs suite_builder =
             12 . min (Decimal.new "13") . should_equal (Decimal.new "12")
 
             Decimal.new "12.1" . min 12.3 . should_equal (Decimal.new "12.1")
-            12.1 . min (Decimal.new "12.3") . should_equal (Decimal.from 12.1)
+            assert_eq_is_true (12.1 . min (Decimal.new "12.3")) (Decimal.from 12.1)
 
             Decimal.new "0" . min (Decimal.new "-1") . should_equal (Decimal.new "-1")
             0 . min (Decimal.new "-1") . should_equal (Decimal.new "-1")

[JaroslavTulach]

• the reproducer indicates the problem is in EqualsNode implementation
• we knew about the deficiencies for ages
  • for example this Heisenbug in EqualsNode Specializations #5758 report
• but we never found enough time to tackle them

Hypothesis

• the problem is in the order of specializations to handle various combination of types
  • it is not really happening in production, as usually the set of types is limited
  • however with . should_equal it is different!
  • there is == invocation inside of .should_equals and
  • it is being fed with all possible types during the execution of test/Base_Tests
• why it happens only on Oracle GraalVM?
  • when the amount of specializations gets too big, the node splits
  • e.g. a new clone of the EqualsNode is created and specializations start from scratch
  • the behavior of splitting is different between GraalVM Comunity and Oracle GraalVM
• why it happens only sometimes?
  • the splitting is driven compiler/Truffle/JVM decisions and is non-deterministic
  • only sometimes the splitting happens in the "necessary moment"
  • which then leads to the sequence of specializations that gets us to wrong state

What is needed right now?

• I tried to debug the problem, but unsuccessfully so far
  • the problem is that I need to debug native image and that's not completely easy (using gdb attach right now)
  • debugging could reveal the state the EqualsNode is at
  • as there are state_0 and state_1 fields in EqualsNodeGen
  • that knowing the state could tell us how to write a unit test in EqualsTest
• the other option is to just completely rework EqualsNode as it is too complex
• it has too many @Specializations apparently without exclusive guards
• hence the order matter - ideally the order shouldn't matter