ZJIT: guard_shape_failure happens too frequently on ruby-bench

[k0kubun]

guard_shape_failure side-exits happen way too frequently for benchmarks like optcarrot #715 and liquid-render #714.

It's also now fairly common on benchmarks like lobsters too.

We should:

1. Investigate how guard_shape_failure happens on benchmarks. Try --zjit-trace-exits?
2. Address common guard_shape_failure patterns.
   • Is call-threshold or num-profiles too low to profile those shapes?
   • Should we recompile the ISEQ if this happens?

[aidenfoxivey]

I would be interested to take a look into this.

I'm trying to get a sense of what all of it means. I think that guard_shape_failure is a side exit that must happen when the shape_id of an object doesn't match some expected shape_id. I think shape_id refers to an integer that is a bitmask for some sort of characteristic about a given object.

[k0kubun]

Objects have the same shape_id when they have inserted the same instance variables in the same order. When a shape_id mismatch happens, it has a different set of instance variables or inserted them in a different order.

In ZJIT, we try to prevent these exits from happening by having 5 profiling cycles. I haven't looked into how these profiles interact with HIR (that's part of this issue's job), but if we can observe a pattern in those profiles that would lead to shape_id mismatches, one way to fix it would be to stop optimizing GetIvar into LoadIvar* in that case.

[aidenfoxivey]

Ah, so this issue would be related to

    /// Run all the optimization passes we have.
    pub fn optimize(&mut self) {
        // Function is assumed to have types inferred already
        self.optimize_direct_sends();
        #[cfg(debug_assertions)] self.assert_validates();
        self.optimize_getivar();
        #[cfg(debug_assertions)] self.assert_validates();
        self.optimize_c_calls();
        #[cfg(debug_assertions)] self.assert_validates();
        self.optimize_ranges();
        #[cfg(debug_assertions)] self.assert_validates();
        self.fold_constants();
        #[cfg(debug_assertions)] self.assert_validates();
        self.clean_cfg();
        #[cfg(debug_assertions)] self.assert_validates();
        self.eliminate_dead_code();
        #[cfg(debug_assertions)] self.assert_validates();
    }

[tekknolagi]

It would be good to get new numbers now that we instead do 30 profile executions

[k0kubun]

This was filed after the threshold change ruby#14410 and both issues have numbers after that too #715 (comment) #714 (comment).

[tekknolagi]

Call threshold 5:

Top-11 side exit reasons (100.0% of total 9,390,071):
                        compile_error: 2,791,858 (29.7%)
                  guard_shape_failure: 2,373,022 (25.3%)
                      unhandled_kwarg: 1,483,319 (15.8%)
                   guard_type_failure: 1,299,682 (13.8%)
                      unhandled_splat: 1,015,000 (10.8%)
                   optional_arguments:   261,354 ( 2.8%)
                   unhandled_hir_insn:   107,402 ( 1.1%)
                  unhandled_yarv_insn:    56,697 ( 0.6%)
                           patchpoint:     1,355 ( 0.0%)
               obj_to_string_fallback:       339 ( 0.0%)
  block_param_proxy_not_iseq_or_ifunc:        43 ( 0.0%)

Call threshold 30:

Top-12 side exit reasons (100.0% of total 8,991,400):
                        compile_error: 2,688,620 (29.9%)
                  guard_shape_failure: 2,450,224 (27.3%)
                      unhandled_splat: 1,461,387 (16.3%)
                      unhandled_kwarg: 1,384,663 (15.4%)
                   guard_type_failure:   605,135 ( 6.7%)
                   optional_arguments:   261,045 ( 2.9%)
                   unhandled_hir_insn:    82,330 ( 0.9%)
                  unhandled_yarv_insn:    56,594 ( 0.6%)
                           patchpoint:     1,044 ( 0.0%)
               obj_to_string_fallback:       314 ( 0.0%)
  block_param_proxy_not_iseq_or_ifunc:        43 ( 0.0%)
                            interrupt:         1 ( 0.0%)

So not much has changed. Interestingly, disabling monomorphic ivar caching seems not to change performance very much. This might either indicate that there is not much getivar (2MM compared to 12MM uncached send) or that we're just always failing, so the specialized path + side exit is ~the same cost as uncached path.

[tekknolagi]

This was filed after the threshold change

Ah, ok, I read

In ZJIT, we try to prevent these exits from happening by having 5 profiling cycles.

and was a little confused. Thanks!

[k0kubun]

Yeah, those 5 profiles did help the situation in many benchmarks, so it led to the explanation. We just need something else to make it better than that.

[k0kubun]

#847 should help this issue too.

[k0kubun]

ruby#16589 should have fixed most of them