diff --git a/NEWS.md b/NEWS.md index 56351a5c2cbcfd..bd7a2ec546af0d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -242,12 +242,15 @@ A lot of work has gone into making Ractors more stable, performant, and usable. [Feature #15330]: https://bugs.ruby-lang.org/issues/15330 [Feature #21390]: https://bugs.ruby-lang.org/issues/21390 [Feature #21768]: https://bugs.ruby-lang.org/issues/21768 +[Feature #21781]: https://bugs.ruby-lang.org/issues/21781 [Feature #21785]: https://bugs.ruby-lang.org/issues/21785 [Feature #21796]: https://bugs.ruby-lang.org/issues/21796 [Feature #21853]: https://bugs.ruby-lang.org/issues/21853 [Feature #21861]: https://bugs.ruby-lang.org/issues/21861 [Feature #21932]: https://bugs.ruby-lang.org/issues/21932 [Feature #21981]: https://bugs.ruby-lang.org/issues/21981 +[Feature #22097]: https://bugs.ruby-lang.org/issues/22097 +[Feature #22135]: https://bugs.ruby-lang.org/issues/22135 [Feature #22137]: https://bugs.ruby-lang.org/issues/22137 [Feature #22139]: https://bugs.ruby-lang.org/issues/22139 [Feature #22175]: https://bugs.ruby-lang.org/issues/22175 diff --git a/cont.c b/cont.c index 2c4fafac8e4c75..252e2ec199036d 100644 --- a/cont.c +++ b/cont.c @@ -2646,6 +2646,9 @@ rb_fiber_start(rb_fiber_t *fiber) th->blocking += 1; } + /* resolved before EC_PUSH_TAG to keep the setjmp region minimal */ + const rb_cref_t *cref = rb_proc_refinements_cref(fiber->first_proc); + EC_PUSH_TAG(th->ec); if ((state = EC_EXEC_TAG()) == TAG_NONE) { rb_context_t *cont = &VAR_FROM_MEMORY(fiber)->cont; @@ -2659,8 +2662,7 @@ rb_fiber_start(rb_fiber_t *fiber) th->ec->root_svar = Qfalse; EXEC_EVENT_HOOK(th->ec, RUBY_EVENT_FIBER_SWITCH, th->self, 0, 0, 0, Qnil); - cont->value = rb_vm_invoke_proc(th->ec, proc, argc, argv, cont->kw_splat, VM_BLOCK_HANDLER_NONE, - rb_proc_refinements_cref(fiber->first_proc)); + cont->value = rb_vm_invoke_proc(th->ec, proc, argc, argv, cont->kw_splat, VM_BLOCK_HANDLER_NONE, cref); } EC_POP_TAG(); diff --git a/depend b/depend index 8db2d7a6759f4f..fc1960615068c6 100644 --- a/depend +++ b/depend @@ -17920,6 +17920,7 @@ string.$(OBJEXT): {$(VPATH)}vm_core.h string.$(OBJEXT): {$(VPATH)}vm_debug.h string.$(OBJEXT): {$(VPATH)}vm_opts.h string.$(OBJEXT): {$(VPATH)}vm_sync.h +string.$(OBJEXT): {$(VPATH)}zjit.h strlcat.$(OBJEXT): {$(VPATH)}config.h strlcat.$(OBJEXT): {$(VPATH)}internal/attr/format.h strlcat.$(OBJEXT): {$(VPATH)}internal/compiler_is.h diff --git a/string.c b/string.c index a503a2d505e436..5339b9ac0b5d15 100644 --- a/string.c +++ b/string.c @@ -49,6 +49,7 @@ #include "ruby_assert.h" #include "shape.h" #include "vm_sync.h" +#include "zjit.h" #include "ruby/internal/attr/nonstring.h" #if defined HAVE_CRYPT_R @@ -2037,6 +2038,42 @@ rb_ec_str_resurrect(struct rb_execution_context_struct *ec, VALUE str, bool chil return new_str; } +#if USE_ZJIT +bool +rb_zjit_str_resurrect_fastpath(VALUE str, bool chilled, size_t *size_out, + VALUE *flags_out, + long *len_out, size_t *byte_size_out) +{ + if (chilled && RTEST(rb_ivar_defined(str, id_debug_created_info))) return false; + + if (!STR_EMBED_P(str)) return false; + + long len = RSTRING_LEN(str); + long termlen = TERM_LEN(str); + size_t size = rb_str_embed_size(len + termlen, 0); + if (!rb_gc_size_allocatable_p(size)) return false; + + VALUE flags = FL_TEST_RAW(str, flag_mask); + + if ((flags & ENCODING_MASK) == ((VALUE)ENCODING_INLINE_MAX << ENCODING_SHIFT)) { + return false; + } + + flags &= ~FL_FREEZE; + flags |= T_STRING; + if (chilled) flags |= STR_CHILLED; + + shape_id_t shape_id = rb_shape_transition_slot_size(ROOT_SHAPE_ID | SHAPE_ID_LAYOUT_OTHER, + rb_gc_size_slot_size(size)); + + *size_out = size; + *flags_out = flags | ((VALUE)shape_id << SHAPE_FLAG_SHIFT); + *len_out = len; + *byte_size_out = (size_t)(len + termlen); + return true; +} +#endif + VALUE rb_str_with_debug_created_info(VALUE str, VALUE path, int line) { diff --git a/win32/win32.c b/win32/win32.c index 69fc706b2e062a..18476de7a47014 100644 --- a/win32/win32.c +++ b/win32/win32.c @@ -1655,7 +1655,7 @@ has_redirection(const char *cmd, UINT cp) case '%': if (*++ptr != '_' && !ISALPHA(*ptr)) break; while (*++ptr == '_' || ISALNUM(*ptr)); - if (*ptr++ == '%') return TRUE; + if (*ptr && *ptr++ == '%') return TRUE; break; case '\\': diff --git a/zjit.h b/zjit.h index 2bbac39b7c7348..fb189537ba8370 100644 --- a/zjit.h +++ b/zjit.h @@ -94,6 +94,7 @@ void rb_zjit_jit_frame_update_references(zjit_jit_frame_t *jit_frame); void rb_zjit_materialize_frames(const rb_execution_context_t *ec, rb_control_frame_t *cfp); size_t rb_zjit_hash_new_size(void); bool rb_zjit_class_allocate_instance_fastpath(VALUE klass, size_t *size_out, shape_id_t *shape_id_out); +bool rb_zjit_str_resurrect_fastpath(VALUE str, bool chilled, size_t *size_out, VALUE *flags_out, long *len_out, size_t *byte_size_out); // Special value for cfp->jit_return that means "this is a C method frame, use // rb_zjit_c_frame as the JITFrame". We don't control the native stack layout diff --git a/zjit/bindgen/src/main.rs b/zjit/bindgen/src/main.rs index 943ae22e30b708..5892f61e39cedf 100644 --- a/zjit/bindgen/src/main.rs +++ b/zjit/bindgen/src/main.rs @@ -113,6 +113,7 @@ fn main() { .allowlist_function("rb_zjit_insn_to_bare_insn") .allowlist_function("rb_zjit_hash_new_size") .allowlist_function("rb_zjit_class_allocate_instance_fastpath") + .allowlist_function("rb_zjit_str_resurrect_fastpath") // For crashing .allowlist_function("rb_bug") diff --git a/zjit/src/codegen.rs b/zjit/src/codegen.rs index 18f6071cb05399..a20ce27af512dd 100644 --- a/zjit/src/codegen.rs +++ b/zjit/src/codegen.rs @@ -631,7 +631,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio Insn::ArrayLength { array } => gen_array_length(asm, opnd!(array)), Insn::ObjectAlloc { val, state } => gen_object_alloc(jit, asm, function, opnd!(val), &function.frame_state(*state)), &Insn::ObjectAllocClass { class, state } => gen_object_alloc_class(jit, asm, class, &function.frame_state(state)), - Insn::StringCopy { val, chilled, state } => gen_string_copy(asm, opnd!(val), *chilled, &function.frame_state(*state)), + Insn::StringCopy { val, chilled, state } => gen_string_copy(jit, asm, function, *val, opnd!(val), *chilled, &function.frame_state(*state)), Insn::StringConcat { strings, state } => gen_string_concat(jit, asm, function, opnds!(strings), &function.frame_state(*state)), &Insn::StringGetbyte { string, index } => gen_string_getbyte(asm, opnd!(string), opnd!(index)), Insn::StringSetbyteFixnum { string, index, value } => gen_string_setbyte_fixnum(asm, opnd!(string), opnd!(index), opnd!(value)), @@ -2024,12 +2024,74 @@ fn gen_invokesuperforward( ) } +const STR_INLINE_STORE_MAX_BYTES: usize = 128; + /// Compile a string resurrection -fn gen_string_copy(asm: &mut Assembler, recv: Opnd, chilled: bool, state: &FrameState) -> Opnd { +fn gen_string_copy(jit: &mut JITState, asm: &mut Assembler, function: &Function, val_id: InsnId, recv: Opnd, chilled: bool, state: &FrameState) -> Opnd { // TODO: split rb_ec_str_resurrect into separate functions gen_prepare_leaf_call_with_gc(asm, state); - let chilled = if chilled { Opnd::Imm(1) } else { Opnd::Imm(0) }; - asm_ccall!(asm, rb_ec_str_resurrect, EC, recv, chilled) + + let Some(src) = function.type_of(val_id).ruby_object() else { + return asm_ccall!(asm, rb_ec_str_resurrect, EC, recv, (chilled as i64).into()); + }; + + let slow_path = |asm: &mut Assembler| asm_ccall!(asm, rb_ec_str_resurrect, EC, Opnd::Value(src), (chilled as i64).into()); + + let mut alloc_size: usize = 0; + let mut flags: VALUE = VALUE(0); + let mut len: c_long = 0; + let mut byte_size: usize = 0; + let has_fastpath = unsafe { + rb_zjit_str_resurrect_fastpath(src, chilled, &mut alloc_size, &mut flags, &mut len, &mut byte_size) + }; + if !has_fastpath { + return slow_path(asm); + } + + let full_flags = flags.as_u64(); + let klass = unsafe { rb_cString }; + + // Because inline stores are 8 bytes, storing large embedded strings would + // generate a large number of stores (!125 for a string in the 1024b size + // pool). Here we choose an arbitrary threshold (128 bytes, or 16 stores), + // above which we'll emit a C call to memcpy instead of multiple stores. + if byte_size > STR_INLINE_STORE_MAX_BYTES { + return gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, full_flags, klass, + &|asm, obj| { + asm.store(Opnd::mem(VALUE_BITS, obj, RUBY_OFFSET_RSTRING_LEN), Opnd::Imm(len)); + let src_obj = asm.load(Opnd::Value(src)); + let src_ptr = asm.lea(Opnd::mem(64, src_obj, RUBY_OFFSET_RSTRING_AS_ARY)); + let dst_ptr = asm.lea(Opnd::mem(64, obj, RUBY_OFFSET_RSTRING_AS_ARY)); + asm.ccall(memcpy as *const u8, vec![dst_ptr, src_ptr, Opnd::UImm(byte_size as u64)]); + }, + slow_path); + } + + // Pre-process string data into 8 byte chunks and take care of padding + // outside the loop, so we can keep the complexity out of the fast path + // loop. + let padded_size = byte_size.next_multiple_of(8); + let Some(src_bytes) = (unsafe { src.as_rstring_byte_slice() }) else { + return slow_path(asm); + }; + debug_assert_eq!(src_bytes.len(), len as usize); + let mut string_bytes = vec![0u8; padded_size]; + string_bytes[..src_bytes.len()].copy_from_slice(src_bytes); + + gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, full_flags, klass, + &|asm, obj| { + asm.store(Opnd::mem(VALUE_BITS, obj, RUBY_OFFSET_RSTRING_LEN), Opnd::Imm(len)); + for (i, chunk) in string_bytes.chunks_exact(8).enumerate() { + let word = u64::from_le_bytes(chunk.try_into().unwrap()); + let offset = RUBY_OFFSET_RSTRING_AS_ARY + (i as i32) * 8; + asm.store(Opnd::mem(64, obj, offset), Opnd::UImm(word)); + } + }, + slow_path) +} + +unsafe extern "C" { + fn memcpy(dst: *mut c_void, src: *const c_void, n: usize) -> *mut c_void; } fn gen_string_equal(asm: &mut Assembler, left: Opnd, right: Opnd) -> lir::Opnd { @@ -2068,7 +2130,7 @@ fn gen_new_array( let flags = (RUBY_T_ARRAY as u64) | (RARRAY_EMBED_FLAG as u64); let klass = unsafe { rb_cArray }; - gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, klass, |asm| { + gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, klass, &|_asm, _obj| {}, |asm| { asm_ccall!(asm, rb_ec_ary_new_from_values, EC, 0i64.into(), Opnd::UImm(0)) }) } @@ -2361,14 +2423,13 @@ fn gen_new_hash( let flags = RUBY_T_HASH as u64; let klass = unsafe { rb_cHash }; - let hash = gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, klass, |asm| { - asm_ccall!(asm, rb_hash_new,) - }); - // TODO: this runs on the slow path too, where rb_hash_new already set - // ifnone. A fast-path-only init hook in gc_fastpath_new_obj would avoid - // the redundant store and be reusable for other types. - asm.store(Opnd::mem(VALUE_BITS, hash, RUBY_OFFSET_RHASH_IFNONE), Qnil.into()); - hash + gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, klass, + &|asm, hash| { + asm.store(Opnd::mem(VALUE_BITS, hash, RUBY_OFFSET_RHASH_IFNONE), Qnil.into()); + }, + |asm| { + asm_ccall!(asm, rb_hash_new,) + }) // TODO: we should use effects_of for this (we would need to add it). } else if sym_keys { // Symbols hash and compare without running Ruby and those operations never raise so @@ -2381,14 +2442,13 @@ fn gen_new_hash( let flags = RUBY_T_HASH as u64; let klass = unsafe { rb_cHash }; - let hash = gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, klass, |asm| { - asm_ccall!(asm, rb_hash_new_with_size, num_pairs.into()) - }); - // TODO: this runs on the slow path too, where rb_hash_new already set - // ifnone. A fast-path-only init hook in gc_fastpath_new_obj would avoid - // the redundant store and be reusable for other types. - asm.store(Opnd::mem(VALUE_BITS, hash, RUBY_OFFSET_RHASH_IFNONE), Qnil.into()); - hash + gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, klass, + &|asm, hash| { + asm.store(Opnd::mem(VALUE_BITS, hash, RUBY_OFFSET_RHASH_IFNONE), Qnil.into()); + }, + |asm| { + asm_ccall!(asm, rb_hash_new_with_size, num_pairs.into()) + }) } else { asm_ccall!(asm, rb_hash_new_with_size, num_pairs.into()) }; @@ -2450,7 +2510,7 @@ fn gen_object_alloc_class(jit: &mut JITState, asm: &mut Assembler, class: VALUE, }; if has_fastpath { let flags = (RUBY_T_OBJECT as u64) | ((shape_id as u64) << RB_SHAPE_FLAG_SHIFT as u64); - gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, class, |asm| { + gc_fastpath::gc_fastpath_new_obj(jit, asm, alloc_size, flags, class, &|_asm, _obj| {}, |asm| { asm_ccall!(asm, rb_class_allocate_instance, class.into()) }) } else { diff --git a/zjit/src/codegen/gc_fastpath.rs b/zjit/src/codegen/gc_fastpath.rs index 1aacf183781fd5..5efc8032430178 100644 --- a/zjit/src/codegen/gc_fastpath.rs +++ b/zjit/src/codegen/gc_fastpath.rs @@ -74,6 +74,7 @@ pub(super) fn gc_fastpath_new_obj( alloc_size: usize, flags: u64, klass: VALUE, + init: &dyn Fn(&mut Assembler, Opnd), slow_path: impl Fn(&mut Assembler) -> lir::Opnd, ) -> lir::Opnd { let Some(fastpath) = prepare_new_obj_fastpath(alloc_size, flags, klass) else { @@ -90,7 +91,7 @@ pub(super) fn gc_fastpath_new_obj( let result_edge = |v: Opnd| Target::Block(Box::new(lir::BranchEdge { target: result_block, args: vec![v] })); - let obj = emit_new_obj_fastpath(jit, asm, &fastpath, miss_block) + let obj = emit_new_obj_fastpath(jit, asm, &fastpath, init, miss_block) .expect("validated GC fastpath must return an object"); asm.jmp(result_edge(obj)); @@ -145,6 +146,7 @@ fn emit_new_obj_fastpath( jit: &mut JITState, asm: &mut Assembler, prepared: &PreparedNewObjFastpath, + init: &dyn Fn(&mut Assembler, Opnd), miss_block: lir::BlockId, ) -> Option { let miss = Target::Block(Box::new(lir::BranchEdge { @@ -154,10 +156,10 @@ fn emit_new_obj_fastpath( match prepared { PreparedNewObjFastpath::Default(fastpath) => { - emit_default_new_obj_fastpath(jit, asm, fastpath, &miss) + emit_default_new_obj_fastpath(jit, asm, fastpath, init, &miss) } PreparedNewObjFastpath::Mmtk(fastpath) => { - emit_mmtk_new_obj_fastpath(jit, asm, fastpath, &miss) + emit_mmtk_new_obj_fastpath(jit, asm, fastpath, init, &miss) } } } @@ -170,6 +172,7 @@ fn emit_default_new_obj_fastpath( jit: &mut JITState, asm: &mut Assembler, fastpath: &RbGcZjitDefaultNewObjFastpath, + init: &dyn Fn(&mut Assembler, Opnd), miss: &Target, ) -> Option { let cursor_offset: i32 = fastpath.cursor_offset.try_into().ok()?; @@ -200,6 +203,8 @@ fn emit_default_new_obj_fastpath( fastpath.klass.into(), ); + init(asm, cursor); + Some(cursor) } @@ -210,6 +215,7 @@ fn emit_mmtk_new_obj_fastpath( jit: &mut JITState, asm: &mut Assembler, fastpath: &RbGcZjitMmtkNewObjFastpath, + init: &dyn Fn(&mut Assembler, Opnd), miss: &Target, ) -> Option { let objspace_total_allocated_objects_offset: i32 = fastpath @@ -316,6 +322,8 @@ fn emit_mmtk_new_obj_fastpath( fastpath.klass.into(), ); + init(asm, obj); + let mutator = asm.load(Opnd::mem( 64, ractor_cache, diff --git a/zjit/src/codegen_tests.rs b/zjit/src/codegen_tests.rs index e53f265202d351..54ff7bd4f992e9 100644 --- a/zjit/src/codegen_tests.rs +++ b/zjit/src/codegen_tests.rs @@ -3260,6 +3260,87 @@ fn test_object_alloc_gc_stress() { "#), @"[Foo, 3, [:@a, :@b, :@c]]"); } +#[test] +fn test_string_copy_gc_stress() { + eval(r#" + # frozen_string_literal: false + def make = "hello world" + "#); + assert_contains_opcode("make", YARVINSN_dupstring); + assert_snapshot!(assert_compiles(r#" + begin + GC.stress = true + make + s = make + orig = s.dup + s << "!" + [s.class, s, s.frozen?, s.encoding.name, s.length, orig] + ensure + GC.stress = false + end + "#), @r#"[String, "hello world!", false, "UTF-8", 12, "hello world"]"#); +} + +#[test] +fn test_string_copy_large_gc_stress() { + eval(r#" + # frozen_string_literal: false + def make = "the quick brown fox jumps over the lazy dog, the quick brown fox jumps over" + "#); + assert_contains_opcode("make", YARVINSN_dupstring); + assert_snapshot!(assert_compiles(r#" + begin + GC.stress = true + make + s = make + s << "!" + [s.class, s.frozen?, s.length, s.end_with?("!")] + ensure + GC.stress = false + end + "#), @"[String, false, 76, true]"); +} + +#[test] +fn test_string_copy_memcpy_gc_stress() { + eval(r#" + # frozen_string_literal: false + def make = "abcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyzabcdefghijklmnopqrstuvwxyz" + "#); + assert_contains_opcode("make", YARVINSN_dupstring); + assert_snapshot!(assert_compiles(r#" + begin + GC.stress = true + make + s = make + s << "!" + [s.class, s.frozen?, s.length, s.end_with?("!")] + ensure + GC.stress = false + end + "#), @"[String, false, 157, true]"); +} + +#[test] +fn test_string_copy_chilled_gc_stress() { + eval(r#" + def make = "hello world" + "#); + assert_contains_opcode("make", YARVINSN_dupchilledstring); + assert_snapshot!(assert_compiles(r#" + begin + GC.stress = true + make + s = make + orig = s.dup + s << "!" + [s.class, s, s.frozen?, s.encoding.name, s.length, orig] + ensure + GC.stress = false + end + "#), @r#"[String, "hello world!", false, "UTF-8", 12, "hello world"]"#); +} + #[test] fn test_new_hash_nonempty() { eval(r#" diff --git a/zjit/src/cruby_bindings.inc.rs b/zjit/src/cruby_bindings.inc.rs index 232478f1220fb1..97f18434397f39 100644 --- a/zjit/src/cruby_bindings.inc.rs +++ b/zjit/src/cruby_bindings.inc.rs @@ -2277,6 +2277,14 @@ unsafe extern "C" { size_out: *mut usize, shape_id_out: *mut shape_id_t, ) -> bool; + pub fn rb_zjit_str_resurrect_fastpath( + str_: VALUE, + chilled: bool, + size_out: *mut usize, + flags_out: *mut VALUE, + len_out: *mut ::std::os::raw::c_long, + byte_size_out: *mut usize, + ) -> bool; pub fn rb_profile_frames( start: ::std::os::raw::c_int, limit: ::std::os::raw::c_int,