Our current implementation of rb_postponed_job_register suffers from
some safety issues that can lead to interpreter crashes (see bug #1991).
Essentially, the issue is that jobs can be called with the wrong
arguments.
We made two attempts to fix this whilst keeping the promised semantics,
but:
* The first one involved masking/unmasking when flushing jobs, which
was believed to be too expensive
* The second one involved a lock-free, multi-producer, single-consumer
ringbuffer, which was too complex
The critical insight behind this third solution is that essentially the
only user of these APIs are a) internal, or b) profiling gems.
For a), none of the usages actually require variable data; they will
work just fine with the preregistration interface.
For b), generally profiling gems only call a single callback with a
single piece of data (which is actually usually just zero) for the life
of the program. The ringbuffer is complex because it needs to support
multi-word inserts of job & data (which can't be atomic); but nobody
actually even needs that functionality, really.
So, this comit:
* Introduces a pre-registration API for jobs, with a GVL-requiring
rb_postponed_job_prereigster, which returns a handle which can be
used with an async-signal-safe rb_postponed_job_trigger.
* Deprecates rb_postponed_job_register (and re-implements it on top of
the preregister function for compatability)
* Moves all the internal usages of postponed job register
pre-registration
when the RUBY_FREE_ON_SHUTDOWN environment variable is set, manually free memory at shutdown.
Co-authored-by: Nobuyoshi Nakada <nobu@ruby-lang.org>
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
need_major_gc is set when a major GC is required. However, if
gc_stress_no_major is also set, then it will not actually run a major
GC.
For example, the following script will sometimes crash:
```
GC.stress = 1
50000.times.map { [] }
```
With the following message:
```
[BUG] cannot create a new page after major GC
```
The intention of GC.verify_compaction_references is, I believe, to force
every single movable object to be moved, so that it's possible to debug
native extensions which not correctly updating their references to
objects they mark as movable.
To do this, it doubles the number of allocated pages for each size pool,
and sorts the heap pages so that the free ones are swept first; thus,
every object in an old page should be moved into a free slot in one of
the new pages.
This worked fine until movement of objects _between_ size pools during
compaction was implemented. That causes some problems for
verify_compaction_references:
* We were doubling the number of pages in each size pool, but actually
if some objects need to move into a _different_ pool, there's no
guarantee that they'll be enough room in that one.
* It's possible for the sweep & compact cursors to meet in one size pool
before all the objects that want to move into that size pool from
another are processed by the compaction.
You can see these problems by changing some of the movement tests in
test_gc_compact.rb to try and move e.g. 50,000 objects instead of
500; the test is not able to actually move all of the objects in a
single compaction run.
To fix this, we do two things in verify_compaction_references:
* Firstly, we add enough pages to every size pool to make them the same
size. This ensures that their compact cursors will all have space to
move during compaction (even if that means empty pages are
pointlessly compacted)
* Then, we examine every object and determine where it _wants_ to be
compacted into. We use this information to add additional pages to
each size pool to handle all objects which should live there.
With these two changes, we can move arbitrary amounts of objects into
the correct size pool in a single call to verify_compaction_references.
My _motivation_ for performing this work was to try and fix some test
stability issues in test_gc_compact.rb. I now no longer think that we
actually see this particular bug in rubyci.org, but I also think
verify_compaction_references should do what it says on the tin, so it's
worth keeping.
[Bug #20022]
This works like objspace_each_obj, except instead of being called with
the start & end address of each page, it's called with the page
structure itself.
[Bug #20022]
Objects with the same shape must always have the same "embeddedness"
(either embedded or heap allocated) because YJIT assumes so. However,
using remove_instance_variable, it's possible that some objects are
embedded and some are heap allocated because it does not re-embed heap
allocated objects.
This commit changes remove_instance_variable to re-embed Object
instance variables when it becomes small enough.
Embedded shared strings cannot be moved because strings point into the
slot of the shared string. There may be code using the RSTRING_PTR on
the stack, which would pin the string but not pin the shared string,
causing it to move.
When generic instance variable has a shape, it is marked movable. If it
it transitions to too complex, it needs to update references otherwise
it may have incorrect references.
This is required for the same reason that super CC needs it.
See 36023d5cb7.
Reproducer:
def cached_foo_callsite(obj) = obj.foo
class Foo
def foo = :v1
module R
refine Foo do
def foo = :unused
end
end
end
obj = Foo.new
cached_foo_callsite(obj) # set up cc with cme for foo=:v1
class Foo
def foo = :v2
end
GC.start # cme for foo=:v1 collected, if not reachable by cached_foo_callsite
cached_foo_callsite(obj)
[Bug #19994]
On large Ruby applications, shutdown may be slow if a major GC has just
started because rb_objspace_call_finalizer completes the GC.
This commit adds gc_abort which discards the mark stack if during
incremental marking and stops sweeping if during lazy sweeping.
Previously, because gc_update_object_references() did not update the
VALUEs in the too_complex ivar st_table for T_CLASS and T_MODULE
objects, GC compaction could finish with corrupted objects.
- start with `klass`, not too_complex
- GC incremental step marks `klass` and its ivars
- ruby code makes `klass` too_complex
- GC compaction runs and move `klass` ivars, but because `klass` is
too_complex, its ivars are not updated by gc_update_object_references(),
leaving T_NONE or T_MOVED objects in the ivar table.
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
Marking both keys and values versus marking just values is an important
distinction, but previously, gc_update_tbl_refs() and gc_update_table_refs()
had names that were too similar.
The st_table storing ivars for too_complex T_OBJECTs have IDs as keys,
but we were marking the IDs unnecessary previously, maybe due to the
confusing naming.
Previously, it tripped the assert about too_complex in
ROBJECT_IV_CAPACITY(). This fixes double faults for some crashes and
helps with use during development.
Since the callback defined in the objspace module might give up the GVL,
we need to make sure the right cr->mfd value is set back after the GVL
is re-obtained.
The previous implementation was using the pointer given
by `DATA_PTR` in all cases. But in the case of an embedded
TypedData, that pointer is garbage, we need to use RTYPEDDATA_GET_DATA
to get the proper data pointer.
Co-Authored-By: Étienne Barrié <etienne.barrie@gmail.com>
This commit adds a new flag RUBY_TYPED_EMBEDDABLE that allows the data
of a TypedData object to be embedded after the object itself. This will
improve cache locality and allow us to save the 8 byte data pointer.
Co-Authored-By: Jean Boussier <byroot@ruby-lang.org>
Tracks other callinfo that references the same kwargs and frees them when all references are cleared.
[bug #19906]
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
fix memory leak in vm_method
This introduces a unified reference_count to clarify who is referencing a method.
This also allows us to treat the refinement method as the def owner since it counts itself as a reference
Co-authored-by: Peter Zhu <peter@peterzhu.ca>
By compacting into slots with pinned objects first, we improve the
efficiency of compaction. As it is less likely that there will exist
pages containing only pinned objects after compaction. This will
increase the number of free pages left after compaction and enable us to
free them.
This used to be the default compaction method before it was removed
(inadvertently?) during the introduction of auto_compaction.
This commit will sort the pages by the pinned slot count at the start of
a major GC that has been triggered by explicitly calling GC.compact (and
thus setting objspace->flags.during_compaction).
It works using the same method by which we sort the heap by empty slot
count during GC.verify_compaction_references.
Previously it was only being sorted during the verify compaction
references stage - so would only happen during testing.
This commit allows us to sort the heap prior to each explicit GC.compact
run
Koichi Sasada
KJ Tsanaktsidis
Adam Hess
Peter Zhu
Soutaro Matsumoto
Nobuyoshi Nakada
Alan Wu
Aaron Patterson
Jean Boussier
Jean Boussier
KJ Tsanaktsidis
Yusuke Endoh
HParker
Matt Valentine-House