Draft
Conversation
Previously we did a non-atomic read of malloc_increase to determine whether or not we needed to gc on malloc. We also should only need to check the value when we have actually flushed/committed a new increase. Use relaxed load in atomic_sub_nounderflow The previous code did a non-atomic load of val, which would work fine (since the value would only be used for an atomic CAS) but resulted in TSAN errors. This alos adjusts the cas to use relaxed memory model, though I'm not sure that actually makes a difference anywhere. Use relaxed atomics for malloc increase as well Use atomics for loading deferred Use atomics for final slot count Use flag for is_lazy_sweeping WIP: simpler background thread (no-op for now) Adjustments Allow one more thread WIP: getting closer to checking end of sweep condition correctly dequeue usleep Attempt pre-sweep Fix accounting of free slots Sweep anything that !needs_cleanup Add TODO Free some T_OBJECTs in background thread get id2ref working Finish background sweeping before compaction gc_abort() waits for background sweeping to finish Add page->page_lock and lock it when changing freelist Right now only the mutator and the background thread need to lock it. We should re-init all these locks on fork due to Ractors (TODO). Make sure not background sweeping during Process.warmup Less locking/unlocking in gc_sweep_step_worker Better sweep_lock management reinit sweep_lock,sweep_cond after fork Allow sweep thread to acquire VM lock. It never joins a VM barrier. Add simply T_DATA freeing Also finish background freeing in ruby_vm_destruct. Allow taking VM lock in sweep thread Add fiber_pool_lock for cont.c Don't take VM lock in background sweep thread Make id2ref_tbl_lock re-entrant We can get the following: 1) RB_VM_LOCK() // need this to allow GC when inserting into tbl 2) id2ref_tbl_lock() // 3) insert into id2ref_tbl, causes GC 4) free object id, which acquires id2ref_tbl_lock again Therefore, the lock needs to be re-entrant. freeing of id2ref object_id in background thread Get gen fields freeing done in background sweep thread First pass at making zombies in background sweep thread add mutexes_lock lock for thread->keeping_mutexes We need to lock this when manipulating this linked list, because freeing a mutex, which can now be done in a background thread, manipulates it. I made this lock global for now, but it should really be either per-ractor or per-thread. Add autoload_free_lock We can't free autoload_data by 1 thread while also freeing an autoload_const that's associated with it concurrently. This can happen currently if they're on separate pages. Add assertions after major GC that background thread is inactive. I'm going to work on allowing background sweeping during a major (unless explicitly requested via GC.start). This is probably more important even than during minors. Add more assertions and comments whitelist making zombies in sweep thread sweep some imemos in background sweep thread tmp commit stuck tmp Get 1 pass over pages mostly working GC compaction tests are still broken. Not sure why. TODO: when in background thread, never modify the page's freelist directly in case user code is being run. Instead, each page should have a deferred_freelist that the user thread will link in when the page is swept. Merge freelist and deferred freelist when we process a page some cleanup Get GC compaction working, doesn't use background thread Fix running GC in cleanup finalizers stuck with GC compact Fix GC.compact Remove usage of page_lock mutex as we no longer need it. Keep actual lock around, but I'll remove it in a separate commit. GC: Remove unused page->page_lock mutex cleanup Remove unused code, add comments Background thread only sweeps until ruby thread is done with that heap There are some problems with the current approach: 1) The background thread can get ahead of the ruby thread on the current heap and sweep more than is necessary instead of moving on to the next heap. We should track `incremental_step_freed_objects` for each heap so ruby thread and background thread are in sync, and background thread can sweep next heap when necessary. 2) We need to restart the background sweeping when we exit from GC. There should be a `objspace->background sweep_mode` after GC exits and background sweeping begins. checkin Fix issues with parallel sweep Issues were: * post-fork issues * gc_sweep_dequeue_page/heap_is_sweep_done/has_sweeping_pages trio is tricky * rb_ec_cleanup issues (aborting bg sweeping, stopping thread) Fix issue with gc_sweep_rest() that could loop forever It could happen when background sweeping got ahead of the ruby thread. Fix more bugs Attempt to make has_sweeping_pages() faster We can make it even faster if we always let the ruby thread take the last page. This is what it used to do, and I think it was the right strategy in hindsight, just because of `has_sweeping_pages` and `gc_sweep_finish`. Otherwise, the ruby thread could need to wait on the background thread somtimes when it's called. Simplify end conditions by ruby thread taking last sweeping_page This is how it used to work, and I think it's a good idea to simplify checks for when sweeping is finished. Tracking down allocation bug Fixed allocation bug It had to do with adjacent bitfields being same memory object and used concurrently. Changed them to bools and it fixed the issue. Improve efficiency of requesting background sweep help Keep track of heap->latest_swept_page cleanup unlink pages in sweep thread Add to free_pages and pooled_pages in sweep thread remove redundant work Use atomic for heap->foreground_sweep_steps and separate swept_pages lock Add heap->skip_sweep_continue Add parallel sweep lock stats Output sweep time at end of process Add counts of sweep events less conditionals Change PSWEEP_LOCK_STATS to use per-callsite stats Add wall clock psweep timings first pass at rb_garbage_object_p with sweep thread Fix WB issues with sweep thread Use atomic operations for bitmaps that can be read/modified by both the mutator and the sweep thread. Avoid the tricky case of `gc_setup_mark_bits` by deferring it to sweep finish. This way, it doesn't conflict with write barriers. Make page->needs_setup_mark_bits its own memory object tmp commit before pairing Bug fix for mark T_NONE John found the fix.
Add this flag to all internal types. Internal extension types are skipped for now.
This is so that garbage_object_p() will work correctly.
…ally The use here is not protected by the sweep lock, so we should. Also, use atomic load for checking if object is marked so that it's not re-ordered past the next atomic load of page->before_sweep.
Make popcount_bits work for all platforms
Also, don't call GET_EC() for every call to free_vm_weak_references. We know in advance when this is called from the sweep thread, so just call the correct function in pre_sweep_plane.
We no longer need it because we changed how zombies are handled.
Since MMTK might also use this, called it something more generic. It's now `rb_gc_obj_free_concurrency_safe_vm_weak_references`.
|
It also works with RGENGC_CHECK_MODE=1
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.