diff options
| author | Alan Wu <[email protected]> | 2022-11-29 16:14:13 -0500 |
|---|---|---|
| committer | Maxime Chevalier-Boisvert <[email protected]> | 2022-11-30 12:23:50 -0500 |
| commit | a0b0365e905e1ac51998ace7e6fc723406a2f157 () | |
| tree | 5523f45099f3a5e2c8cf3112f4acc4cf42d5defe | |
| parent | b30248f74a8f6ce37a78f07597c7c5452ff50abd (diff) | |
YJIT: Deallocate `struct Block` to plug memory s
Previously we essentially never freed block even after invalidation. Their reference count never reached zero for a couple of reasons: 1. `Branch::block` formed a cycle with the block holding the branch 2. Strong count on a branch that has ever contained a stub never reached 0 because we increment the `.clone()` call for `BranchRef::into_raw()` didn't have a matching decrement. It's not safe to immediately deallocate blocks during invalidation since `branch_stub_hit()` can end up running with a branch pointer from an invalidated branch. To plug the s, we wait until code GC or global invalidation and deallocate the blocks for iseqs that are definitely not running.
Notes: Merged: https://.com/ruby/ruby/pull/6833
| -rw-r--r-- | bootstraptest/test_yjit.rb | 13 | ||||
| -rw-r--r-- | yjit/src/core.rs | 67 | ||||
| -rw-r--r-- | yjit/src/invariants.rs | 31 |
3 files changed, 84 insertions, 27 deletions
@@ -3423,3 +3423,16 @@ assert_equal '1', %q{ bar { } bar { } } @@ -1,4 +1,3 @@ -//use crate::asm::x86_64::*; use crate::asm::*; use crate::backend::ir::*; use crate::codegen::*; @@ -17,6 +16,7 @@ use std::mem; use std::rc::{Rc}; use YARVOpnd::*; use TempMapping::*; // Maximum number of temp value types we keep track of pub const MAX_TEMP_TYPES: usize = 8; @@ -492,6 +492,10 @@ pub struct IseqPayload { // Indexes of code pages used by this this ISEQ pub pages: HashSet<usize>, } impl IseqPayload { @@ -599,8 +603,6 @@ pub extern "C" fn rb_yjit_iseq_free(payload: *mut c_void) { } }; - use crate::invariants; - // Take ownership of the payload with Box::from_raw(). // It drops right before this function returns. // SAFETY: We got the pointer from Box::into_raw(). @@ -609,10 +611,10 @@ pub extern "C" fn rb_yjit_iseq_free(payload: *mut c_void) { // Increment the freed iseq count incr_counter!(freed_iseq_count); - // Remove all blocks in the payload from global invariants table. for versions in &payload.version_map { for block in versions { - invariants::block_assumptions_free(&block); } } } @@ -1896,10 +1898,12 @@ fn set_branch_target( // Generate an outlined stub that will call branch_stub_hit() let stub_addr = ocb.get_write_ptr(); - // Get a raw pointer to the branch while keeping the reference count alive - // Here clone increments the strong count by 1 - // This means the branch stub owns its own reference to the branch let branch_ptr: *const RefCell<Branch> = BranchRef::into_raw(branchref.clone()); let mut asm = Assembler::new(); asm.comment("branch stub hit"); @@ -2087,12 +2091,7 @@ pub fn defer_compilation( asm.mark_branch_end(&branch_rc); } -// Remove all references to a block then free it. -pub fn free_block(blockref: &BlockRef) { - use crate::invariants::*; - - block_assumptions_free(blockref); - let block = blockref.borrow(); // Remove this block from the predecessor's targets @@ -2123,6 +2122,19 @@ pub fn free_block(blockref: &BlockRef) { } } } // No explicit deallocation here as blocks are ref-counted. } @@ -2293,7 +2305,7 @@ pub fn invalidate_block_version(blockref: &BlockRef) { // FIXME: // Call continuation addresses on the stack can also be atomically replaced by jumps going to the stub. - free_block(blockref); ocb.unwrap().mark_all_executable(); cb.mark_all_executable(); @@ -2301,6 +2313,31 @@ pub fn invalidate_block_version(blockref: &BlockRef) { incr_counter!(invalidation_count); } #[cfg(test)] mod tests { use crate::core::*; @@ -497,21 +497,28 @@ pub extern "C" fn rb_yjit_tracing_invalidate_all() { // Stop other ractors since we are going to machine code. with_vm_lock(src_loc!(), || { // Make it so all live block versions are no longer valid branch targets for_each_iseq(|iseq| { if let Some(payload) = get_iseq_payload(iseq) { - // C comment: - // ing the blocks for now since we might have situations where - // a different ractor is waiting for the VM lock in branch_stub_hit(). - // If we free the block that ractor can wake up with a dangling block. - // - // Deviation: since we ref count the the blocks now, we might be deallocating and - // not the block. - // - // Empty all blocks on the iseq so we don't compile new blocks that jump to the - // invalidated region. let blocks = payload.take_all_blocks(); - for blockref in blocks { - block_assumptions_free(&blockref); } } |