Validate CodeInstances with no external edges

[timholy]

In #38983 and #41872, it was discovered that only CodeInstances with
external backedges get validated after deserialization.
This stores new CodeInstances in a list, and if they have neither been
validated nor invalidated, it validates them.

Closes #41872

CC @martinholters

[vtjnash]

I think the serializer needs to store this information. Sometimes we will have methods without backedges because we know that they are invalid beforehand.

[vtjnash]

I think this seems reasonable to me

[vtjnash]

hm, CI might disagree with it: https://build.julialang.org/#/builders/7/builds/2647

[timholy]

Oh dear. If we need to revert that's OK, but I'm looking into this.

[timholy]

Hmm, strange. It's rare, but if I run the test ~50 times I can get it. From debugging with a key diff

@@ -2083,10 +2089,17 @@ static void validate_new_code_instances(void)
     size_t i;
     for (i = 0; i < new_code_instance_validate.size; i += 2) {
         if (new_code_instance_validate.table[i+1] != HT_NOTFOUND) {
+         jl_printf(JL_STDOUT, "validating ");
+         jl_(((jl_code_instance_t*)new_code_instance_validate.table[i])->def);
             ((jl_code_instance_t*)new_code_instance_validate.table[i])->max_world = ~(size_t)0;
             new_code_instance_validate.table[i+1] = HT_NOTFOUND;
-        }
+        } else if (new_code_instance_validate.table[i] != HT_NOTFOUND) {
+         jl_printf(JL_STDOUT, "for invalid ci, max_world is %ld: ", ((jl_code_instance_t*)new_code_instance_validate.table[i])->max_world);
+         jl_(((jl_code_instance_t*)new_code_instance_validate.table[i])->def);
+       }
     }

I captured

invalidate: g() from g()
validating (::Type{Foo4b3a94a1a081a8cb.MyType{:sym}})() from (::Type{Foo4b3a94a1a081a8cb.MyType{T}})() where {T}
validating (::Type{Foo4b3a94a1a081a8cb.Result})(Base.Missing) from (::Type{Foo4b3a94a1a081a8cb.Result})(Union{Base.Missing, Int64})
for invalid ci, max_world is -1: call_bottom() from call_bottom()
for invalid ci, max_world is 0: g() from g()
validating g() from g()
precompile (1) |         failed at 2021-08-25T15:53:40.961
Test Failed at /home/tim/src/julia-branch/test/precompile.jl:218
  Expression: Foo.g() === 97.0
   Evaluated: 2.0 === 97.0


Test Summary: | Pass  Fail  Total
  Overall     |    7     1      8
    precompile |    7     1      8
    FAILURE

It makes it seem as if it could be in the htable twice. Is that possible? Do I have to do something to ensure the validity of the table?

[vtjnash]

Seems like a case you can quickly catch under rr, then be able to quickly pinpoint when each of those two got added to the cache

[timholy]

I am guessing this will fix it:

diff --git a/src/dump.c b/src/dump.c
index af3290a511..0cd8513426 100644
--- a/src/dump.c
+++ b/src/dump.c
@@ -2080,12 +2080,20 @@ static void jl_insert_backedges(jl_array_t *list, jl_array_t *targets)
 
 static void validate_new_code_instances(void)
 {
-    size_t i;
-    for (i = 0; i < new_code_instance_validate.size; i += 2) {
+    size_t i = 0;
+    while (i < new_code_instance_validate.size) {
         if (new_code_instance_validate.table[i+1] != HT_NOTFOUND) {
             ((jl_code_instance_t*)new_code_instance_validate.table[i])->max_world = ~(size_t)0;
             new_code_instance_validate.table[i+1] = HT_NOTFOUND;
-        }
+        } else {
+           // The same key can be inserted in multiple places
+           void *p = new_code_instance_validate.table[i];
+            while (i + 2 < new_code_instance_validate.size && new_code_instance_validate.table[i+2] == p) {
+                i += 2;
+                new_code_instance_validate.table[i+1] = HT_NOTFOUND;
+            }
+       }
+       i += 2;
     }
 }

The same pointer is definitely inserted twice in a row (rarely).

[timholy]

Or would it be better to call ptrhash_get instead of relying on the i+1 value?

[timholy]

See #42008