Skip to content

Commit 1083c2c

Browse files
authored
ZJIT: Add stats for cfuncs that are not optimized (ruby#14638)
* ZJIT: Add stats for cfuncs that are not optimized * ZJIT: Add IncrCounterPtr HIR instead From `lobsters` ``` Top-20 Unoptimized C functions (73.0% of total 15,276,688): Kernel#is_a?: 2,052,363 (13.4%) Class#current: 1,892,623 (12.4%) String#to_s: 975,973 ( 6.4%) Hash#key?: 677,623 ( 4.4%) String#empty?: 636,468 ( 4.2%) TrueClass#===: 457,232 ( 3.0%) Hash#[]=: 455,908 ( 3.0%) FalseClass#===: 448,798 ( 2.9%) ActiveSupport::OrderedOptions#_get: 377,468 ( 2.5%) Kernel#kind_of?: 339,551 ( 2.2%) Kernel#dup: 329,371 ( 2.2%) String#==: 324,286 ( 2.1%) String#include?: 297,528 ( 1.9%) Hash#[]: 294,561 ( 1.9%) Array#include?: 287,145 ( 1.9%) Kernel#block_given?: 283,633 ( 1.9%) BasicObject#!=: 278,874 ( 1.8%) Hash#delete: 250,951 ( 1.6%) Set#include?: 246,447 ( 1.6%) NilClass#===: 242,776 ( 1.6%) ``` From `liquid-render` ``` Top-20 Unoptimized C functions (99.8% of total 5,195,549): Hash#key?: 2,459,048 (47.3%) String#to_s: 1,119,758 (21.6%) Set#include?: 799,469 (15.4%) Kernel#is_a?: 214,223 ( 4.1%) Integer#<<: 171,073 ( 3.3%) Integer#/: 127,622 ( 2.5%) CGI::EscapeExt#escapeHTML: 56,971 ( 1.1%) Regexp#===: 50,008 ( 1.0%) String#empty?: 43,990 ( 0.8%) String#===: 36,838 ( 0.7%) String#==: 21,309 ( 0.4%) Time#strftime: 21,251 ( 0.4%) String#strip: 15,271 ( 0.3%) String#scan: 13,753 ( 0.3%) String#+@: 12,603 ( 0.2%) Array#include?: 8,059 ( 0.2%) String#+: 5,295 ( 0.1%) String#dup: 4,606 ( 0.1%) String#-@: 3,213 ( 0.1%) Class#generate: 3,011 ( 0.1%) ```
1 parent 7a18738 commit 1083c2c

File tree

5 files changed

+68
-22
lines changed

5 files changed

+68
-22
lines changed

zjit.rb

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ def stats_string
4343
print_counters_with_prefix(prefix: 'dynamic_send_type_', prompt: 'dynamic send types', buf:, stats:, limit: 20)
4444
print_counters_with_prefix(prefix: 'unspecialized_def_type_', prompt: 'send fallback unspecialized def_types', buf:, stats:, limit: 20)
4545
print_counters_with_prefix(prefix: 'send_fallback_', prompt: 'dynamic send types', buf:, stats:, limit: 20)
46+
print_counters_with_prefix(prefix: 'not_optimized_cfuncs_', prompt: 'unoptimized sends to C functions', buf:, stats:, limit: 20)
4647

4748
# Show exit counters, ordered by the typical amount of exits for the prefix at the time
4849
print_counters_with_prefix(prefix: 'unhandled_yarv_insn_', prompt: 'unhandled YARV insns', buf:, stats:, limit: 20)

zjit/src/codegen.rs

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -419,6 +419,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
419419
Insn::GetSpecialSymbol { symbol_type, state: _ } => gen_getspecial_symbol(asm, *symbol_type),
420420
Insn::GetSpecialNumber { nth, state } => gen_getspecial_number(asm, *nth, &function.frame_state(*state)),
421421
&Insn::IncrCounter(counter) => no_output!(gen_incr_counter(asm, counter)),
422+
Insn::IncrCounterPtr { counter_ptr } => no_output!(gen_incr_counter_ptr(asm, *counter_ptr)),
422423
Insn::ObjToString { val, cd, state, .. } => gen_objtostring(jit, asm, opnd!(val), *cd, &function.frame_state(*state)),
423424
&Insn::CheckInterrupts { state } => no_output!(gen_check_interrupts(jit, asm, &function.frame_state(state))),
424425
&Insn::HashDup { val, state } => { gen_hash_dup(asm, opnd!(val), &function.frame_state(state)) },
@@ -1528,15 +1529,20 @@ fn gen_guard_bit_equals(jit: &mut JITState, asm: &mut Assembler, val: lir::Opnd,
15281529
val
15291530
}
15301531

1532+
/// Generate code that records unoptimized C functions if --zjit-stats is enabled
1533+
fn gen_incr_counter_ptr(asm: &mut Assembler, counter_ptr: *mut u64) {
1534+
if get_option!(stats) {
1535+
let ptr_reg = asm.load(Opnd::const_ptr(counter_ptr as *const u8));
1536+
let counter_opnd = Opnd::mem(64, ptr_reg, 0);
1537+
asm.incr_counter(counter_opnd, Opnd::UImm(1));
1538+
}
1539+
}
1540+
15311541
/// Generate code that increments a counter if --zjit-stats
15321542
fn gen_incr_counter(asm: &mut Assembler, counter: Counter) {
15331543
if get_option!(stats) {
15341544
let ptr = counter_ptr(counter);
1535-
let ptr_reg = asm.load(Opnd::const_ptr(ptr as *const u8));
1536-
let counter_opnd = Opnd::mem(64, ptr_reg, 0);
1537-
1538-
// Increment and store the updated value
1539-
asm.incr_counter(counter_opnd, Opnd::UImm(1));
1545+
gen_incr_counter_ptr(asm, ptr);
15401546
}
15411547
}
15421548

zjit/src/hir.rs

Lines changed: 39 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -711,6 +711,9 @@ pub enum Insn {
711711
/// Increment a counter in ZJIT stats
712712
IncrCounter(Counter),
713713

714+
/// Increment a counter in ZJIT stats for the given counter pointer
715+
IncrCounterPtr { counter_ptr: *mut u64 },
716+
714717
/// Equivalent of RUBY_VM_CHECK_INTS. Automatically inserted by the compiler before jumps and
715718
/// return instructions.
716719
CheckInterrupts { state: InsnId },
@@ -724,7 +727,7 @@ impl Insn {
724727
| Insn::IfTrue { .. } | Insn::IfFalse { .. } | Insn::EntryPoint { .. } | Insn::Return { .. }
725728
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::ArrayExtend { .. }
726729
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetGlobal { .. }
727-
| Insn::SetLocal { .. } | Insn::Throw { .. } | Insn::IncrCounter(_)
730+
| Insn::SetLocal { .. } | Insn::Throw { .. } | Insn::IncrCounter(_) | Insn::IncrCounterPtr { .. }
728731
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } => false,
729732
_ => true,
730733
}
@@ -978,6 +981,7 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
978981
}
979982
Ok(())
980983
},
984+
Insn::IncrCounterPtr { .. } => write!(f, "IncrCounterPtr"),
981985
Insn::Snapshot { state } => write!(f, "Snapshot {}", state.print(self.ptr_map)),
982986
Insn::Defined { op_type, v, .. } => {
983987
// op_type (enum defined_type) printing logic from iseq.c.
@@ -1385,6 +1389,7 @@ impl Function {
13851389
| EntryPoint {..}
13861390
| LoadPC
13871391
| LoadSelf
1392+
| IncrCounterPtr {..}
13881393
| IncrCounter(_)) => result.clone(),
13891394
&Snapshot { state: FrameState { iseq, insn_idx, pc, ref stack, ref locals } } =>
13901395
Snapshot {
@@ -1534,7 +1539,7 @@ impl Function {
15341539
| Insn::IfTrue { .. } | Insn::IfFalse { .. } | Insn::Return { .. } | Insn::Throw { .. }
15351540
| Insn::PatchPoint { .. } | Insn::SetIvar { .. } | Insn::ArrayExtend { .. }
15361541
| Insn::ArrayPush { .. } | Insn::SideExit { .. } | Insn::SetLocal { .. } | Insn::IncrCounter(_)
1537-
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } =>
1542+
| Insn::CheckInterrupts { .. } | Insn::GuardBlockParamProxy { .. } | Insn::IncrCounterPtr { .. } =>
15381543
panic!("Cannot infer type of instruction with no output: {}", self.insns[insn.0]),
15391544
Insn::Const { val: Const::Value(val) } => Type::from_value(*val),
15401545
Insn::Const { val: Const::CBool(val) } => Type::from_cbool(*val),
@@ -2154,9 +2159,9 @@ impl Function {
21542159
self_type: Type,
21552160
send: Insn,
21562161
send_insn_id: InsnId,
2157-
) -> Result<(), ()> {
2162+
) -> Result<(), Option<*const rb_callable_method_entry_struct>> {
21582163
let Insn::SendWithoutBlock { mut recv, cd, mut args, state, .. } = send else {
2159-
return Err(());
2164+
return Err(None);
21602165
};
21612166

21622167
let call_info = unsafe { (*cd).ci };
@@ -2168,20 +2173,20 @@ impl Function {
21682173
(class, None)
21692174
} else {
21702175
let iseq_insn_idx = fun.frame_state(state).insn_idx;
2171-
let Some(recv_type) = fun.profiled_type_of_at(recv, iseq_insn_idx) else { return Err(()) };
2176+
let Some(recv_type) = fun.profiled_type_of_at(recv, iseq_insn_idx) else { return Err(None) };
21722177
(recv_type.class(), Some(recv_type))
21732178
};
21742179

21752180
// Do method lookup
2176-
let method = unsafe { rb_callable_method_entry(recv_class, method_id) };
2181+
let method: *const rb_callable_method_entry_struct = unsafe { rb_callable_method_entry(recv_class, method_id) };
21772182
if method.is_null() {
2178-
return Err(());
2183+
return Err(None);
21792184
}
21802185

21812186
// Filter for C methods
21822187
let def_type = unsafe { get_cme_def_type(method) };
21832188
if def_type != VM_METHOD_TYPE_CFUNC {
2184-
return Err(());
2189+
return Err(None);
21852190
}
21862191

21872192
// Find the `argc` (arity) of the C method, which describes the parameters it expects
@@ -2193,15 +2198,15 @@ impl Function {
21932198
//
21942199
// Bail on argc mismatch
21952200
if argc != cfunc_argc as u32 {
2196-
return Err(());
2201+
return Err(Some(method));
21972202
}
21982203

21992204
// Filter for a leaf and GC free function
22002205
use crate::cruby_methods::FnProperties;
22012206
let Some(FnProperties { leaf: true, no_gc: true, return_type, elidable }) =
22022207
ZJITState::get_method_annotations().get_cfunc_properties(method)
22032208
else {
2204-
return Err(());
2209+
return Err(Some(method));
22052210
};
22062211

22072212
let ci_flags = unsafe { vm_ci_flag(call_info) };
@@ -2218,13 +2223,13 @@ impl Function {
22182223
cfunc_args.append(&mut args);
22192224
let ccall = fun.push_insn(block, Insn::CCall { cfun, args: cfunc_args, name: method_id, return_type, elidable });
22202225
fun.make_equal_to(send_insn_id, ccall);
2221-
return Ok(());
2226+
return Ok(())
22222227
}
22232228
}
22242229
// Variadic method
22252230
-1 => {
22262231
if unsafe { rb_zjit_method_tracing_currently_enabled() } {
2227-
return Err(());
2232+
return Err(None);
22282233
}
22292234
// The method gets a pointer to the first argument
22302235
// func(int argc, VALUE *argv, VALUE recv)
@@ -2256,8 +2261,9 @@ impl Function {
22562261
});
22572262

22582263
fun.make_equal_to(send_insn_id, ccall);
2259-
return Ok(());
2264+
return Ok(())
22602265
}
2266+
22612267
// Fall through for complex cases (splat, kwargs, etc.)
22622268
}
22632269
-2 => {
@@ -2267,7 +2273,7 @@ impl Function {
22672273
_ => unreachable!("unknown cfunc kind: argc={argc}")
22682274
}
22692275

2270-
Err(())
2276+
Err(Some(method))
22712277
}
22722278

22732279
for block in self.rpo() {
@@ -2276,8 +2282,23 @@ impl Function {
22762282
for insn_id in old_insns {
22772283
if let send @ Insn::SendWithoutBlock { recv, .. } = self.find(insn_id) {
22782284
let recv_type = self.type_of(recv);
2279-
if reduce_to_ccall(self, block, recv_type, send, insn_id).is_ok() {
2280-
continue;
2285+
match reduce_to_ccall(self, block, recv_type, send, insn_id) {
2286+
Ok(()) => continue,
2287+
Err(Some(cme)) => {
2288+
if get_option!(stats) {
2289+
let owner = unsafe { (*cme).owner };
2290+
let called_id = unsafe { (*cme).called_id };
2291+
let class_name = get_class_name(owner);
2292+
let method_name = called_id.contents_lossy();
2293+
let qualified_method_name = format!("{}#{}", class_name, method_name);
2294+
let unoptimized_cfunc_counter_pointers = ZJITState::get_unoptimized_cfunc_counter_pointers();
2295+
let counter_ptr = unoptimized_cfunc_counter_pointers.entry(qualified_method_name.clone()).or_insert_with(|| Box::new(0));
2296+
let counter_ptr = &mut **counter_ptr as *mut u64;
2297+
2298+
self.push_insn(block, Insn::IncrCounterPtr { counter_ptr });
2299+
}
2300+
}
2301+
_ => {}
22812302
}
22822303
}
22832304
self.push_insn_id(block, insn_id);
@@ -2423,7 +2444,8 @@ impl Function {
24232444
| &Insn::LoadSelf
24242445
| &Insn::GetLocal { .. }
24252446
| &Insn::PutSpecialObject { .. }
2426-
| &Insn::IncrCounter(_) =>
2447+
| &Insn::IncrCounter(_)
2448+
| &Insn::IncrCounterPtr { .. } =>
24272449
{}
24282450
&Insn::PatchPoint { state, .. }
24292451
| &Insn::CheckInterrupts { state }

zjit/src/state.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ use crate::asm::CodeBlock;
88
use crate::options::get_option;
99
use crate::stats::{Counters, ExitCounters};
1010
use crate::virtualmem::CodePtr;
11+
use std::collections::HashMap;
1112

1213
#[allow(non_upper_case_globals)]
1314
#[unsafe(no_mangle)]
@@ -46,6 +47,9 @@ pub struct ZJITState {
4647

4748
/// Trampoline to call function_stub_hit
4849
function_stub_hit_trampoline: CodePtr,
50+
51+
/// Counter pointers for unoptimized C functions
52+
unoptimized_cfunc_counter_pointers: HashMap<String, Box<u64>>,
4953
}
5054

5155
/// Private singleton instance of the codegen globals
@@ -80,6 +84,7 @@ impl ZJITState {
8084
exit_trampoline,
8185
function_stub_hit_trampoline,
8286
exit_trampoline_with_counter: exit_trampoline,
87+
unoptimized_cfunc_counter_pointers: HashMap::new(),
8388
};
8489
unsafe { ZJIT_STATE = Some(zjit_state); }
8590

@@ -138,6 +143,11 @@ impl ZJITState {
138143
&mut ZJITState::get_instance().exit_counters
139144
}
140145

146+
/// Get a mutable reference to unoptimized cfunc counter pointers
147+
pub fn get_unoptimized_cfunc_counter_pointers() -> &'static mut HashMap<String, Box<u64>> {
148+
&mut ZJITState::get_instance().unoptimized_cfunc_counter_pointers
149+
}
150+
141151
/// Was --zjit-save-compiled-iseqs specified?
142152
pub fn should_log_compiled_iseqs() -> bool {
143153
get_option!(log_compiled_iseqs).is_some()

zjit/src/stats.rs

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -384,6 +384,13 @@ pub extern "C" fn rb_zjit_stats(_ec: EcPtr, _self: VALUE, target_key: VALUE) ->
384384
set_stat_f64!(hash, "ratio_in_zjit", 100.0 * zjit_insn_count as f64 / total_insn_count as f64);
385385
}
386386

387+
// Set unoptimized cfunc counters
388+
let unoptimized_cfuncs = ZJITState::get_unoptimized_cfunc_counter_pointers();
389+
for (signature, counter) in unoptimized_cfuncs.iter() {
390+
let key_string = format!("not_optimized_cfuncs_{}", signature);
391+
set_stat_usize!(hash, &key_string, **counter);
392+
}
393+
387394
hash
388395
}
389396

0 commit comments

Comments
 (0)