Sort method signatures before precompile to avoid order dependence. #30095
Conversation
This should help in making ordering-dependent bugs such as #29923 (an error in caching of inferred methods) deterministic, which will make them much easier to find in the future.
| @@ -364,7 +364,7 @@ void jl_print_gc_stats(JL_STREAM *s); | |||
| void jl_gc_reset_alloc_count(void); | |||
| int jl_assign_type_uid(void); | |||
| jl_value_t *jl_cache_type_(jl_datatype_t *type); | |||
| void jl_resort_type_cache(jl_svec_t *c); | |||
| void jl_sort_types(jl_value_t **types, size_t length); | |||
There was a problem hiding this comment.
I'm wondering whether the name I chose here is appropriate. I get the impression that this ordering is canonical in some sense, so jl_sort_types might be ok?
There was a problem hiding this comment.
It's not terribly canonical, mostly since it can't handle all types. It's just used to speed up a lookup. Probably ok to use here though, since the goal is just to make the order more likely to be predictable. However, fuzzing the order is apparently useful for finding bugs. Maybe we should add some kind of option to shuffle it.
There was a problem hiding this comment.
I also found union_sort_cmp, but that gives up when type parameters list is longer than length 3.
Regarding fuzzing, sounds like a reasonable idea though I think that could be a separate PR. Not because it's hard to shuffle, but because I don't want to spend the time testing a good workflow for that (ie, not in bootstrap!) right at the moment.
This should help in making ordering-dependent bugs such as #29923 (an
error in caching of inferred methods) deterministic, which will make
them much easier to find in the future.
Timing this just for good measure, it appears there's only ~5000 elements to be sorted, which costs a paltry ~ 1 ms.
[edit] I should mention that I tested this with the error in #29923, and it makes the compile completely reproducible.