Adding strings to the worklist is useless, because they never contribute to
cycles. The assembly size on x86_64 does not change. This significantly improves
performance in this synthetic benchmark by 33%.
function test($a) {}
$a = new stdClass();
$a->self = $a;
$a->prop1 = str_repeat('a', 10);
$a->prop2 = str_repeat('a', 10);
$a->prop3 = str_repeat('a', 10);
$a->prop4 = str_repeat('a', 10);
$a->prop5 = str_repeat('a', 10);
$a->prop6 = str_repeat('a', 10);
$a->prop7 = str_repeat('a', 10);
$a->prop8 = str_repeat('a', 10);
$a->prop9 = str_repeat('a', 10);
$a->prop10 = str_repeat('a', 10);
for ($i = 0; $i < 10_000_000; $i++) {
test($a);
gc_collect_cycles();
}
This requires adding IS_TYPE_COLLECTABLE to IS_REFERENCE_EX to ensure these
values continue to be pushed onto the stack. Luckily, IS_TYPE_COLLECTABLE is
currently only used in gc_check_possible_root(), where the checked value cannot
be a reference.
Note that this changes the output of gc_collect_cycles(). Non-cyclic, refcounted
values no longer count towards the total reported values collected.
Also, there is some obvious overlap with GH-17130. This change should be good
nonetheless, especially if we can remove the GC_COLLECTABLE(Z_COUNTED_P(zv))
condition in PHP 9 and rely on Z_COLLECTABLE_P() exclusively, given we can
assume an object doesn't become cyclic at runtime anymore.
Closes GH-17194
Fiber switching was disabled during destructor execution due to conflicts
with the garbage collector. This unfortunately introduces a function color
problem: destructors can not call functions that may switch Fibers.
In this change we update the GC so that Fiber switching during GC is safe. In
turn we allow Fiber switching during destrutor execution.
The GC executes destructors in a dedicated Fiber. If a destructor suspends, the
Fiber is owned by userland and a new dedicated Fiber is created to execute the
remaining destructors. Destructor suspension results in a resurection of the
object, which is handled as usual: The object is not considered garbage anymore,
but may be collected in a later run.
When the GC is executed in the main context (not in a Fiber), then destructors
are executed in the main context as well because there is no risk of conflicting
with GC in this case (main context can not suspend).
Fixes GH-11389
Closes GH-13460
This fixes an edge case causing the GC to be triggered repeatedly.
Destructors might add potential garbage to the buffer, so it may happen that num_root it higher than gc_threshold after collection, thus triggering a GC run almost immediately. This can happen by touching enough objects in a destructor, e.g. by iterating over an array. If this happens again in the new run, and the threshold is not updated, the GC may be triggered again.
The edge case requires specific conditions to be triggered and it must happen rarely in practice:
* At least GC_THRESHOLD_TRIGGER (100) objects must be collected during each run for the threshold to not be updated
* At least GC_G(gc_threshold) (initially 10k) objects must be touched (decref'ed to n>0) by any destructor during each run to fill the buffer
The fix is to increase the threshold if GC_G(num_roots) >= GC_G(gc_threshold) after GC. The threshold eventually reaches a point at which the second condition is not met anymore.
The included tests trigger more than 200 GC runs before the fix, and 2 after the fix (dtors always trigger a second run).
A related issue is that zend_gc_check_root_tmpvars() may add potential garbage before the threshold is adjusted, which may trigger GC and exhaust the stack. This is fixed by setting GC_G(active)=1 around zend_gc_check_root_tmpvars().
* Fix GH-13193: Significant performance degradation in 'foreach' starting from PHP 8.2.13 (caused by garbage collection)
* Don't run zend_gc_remove_root_tmpvars() if GC is not active or GC buffer is empty
- running: true if garbage collection is currently running
- protected: true if the garbage collector is protected and root
additions are forbidden
- full: true if the garbage collector buffer size exceeds GC_MAX_BUF_SIZE
- buffer_size: current garbage collector buffer size
Documentation for existing fields:
- runs: the number of times the garbage collector has been run
- collected: the number of objects collected
- threshold: the number of roots in the buffer which will trigger
garbage collection
- roots: the current number of roots in the buffer
Updated manual example output:
array(8) {
["running"]=>
bool(false)
["protected"]=>
bool(false)
["full"]=>
bool(false)
["runs"]=>
int(5)
["collected"]=>
int(100002)
["threshold"]=>
int(50001)
["buffer_size"]=>
int(131072)
["roots"]=>
int(0)
}
Previusly, to redce stack usage GC traversed object properties and array
elements in both directions. At first, it seeked backward from the last
element, to find the last refcounted zval. Then, it traversed elements
forward. As result, we had 2 non-spatial memory accesses for each
object/array and usuall 2 expensive CPU cache misses.
Now all the traversal algorithms are refactored to scan elements only in
forward direction. The number of checks and GC stack usage should be the
same. The order of element processing may be differenr, but this
should not be a problem.
- for packed arrays we store just an array of zvals without keys.
- the elements of packed array are accessible throuf as ht->arPacked[i]
instead of ht->arData[i]
- in addition to general ZEND_HASH_FOREACH_* macros, we introduced similar
familied for packed (ZEND_HASH_PACKED_FORECH_*) and real hashes
(ZEND_HASH_MAP_FOREACH_*)
- introduced an additional family of macros to access elements of array
(packed or real hashes) ZEND_ARRAY_ELEMET_SIZE, ZEND_ARRAY_ELEMET_EX,
ZEND_ARRAY_ELEMET, ZEND_ARRAY_NEXT_ELEMENT, ZEND_ARRAY_PREV_ELEMENT
- zend_hash_minmax() prototype was changed to compare only values
Because of smaller data set, this patch may show performance improvement
on some apps and benchmarks that use packed arrays. (~1% on PHP-Parser)
TODO:
- sapi/phpdbg needs special support for packed arrays (WATCH_ON_BUCKET).
- zend_hash_sort_ex() may require converting packed arrays to hash.
We partially fixed this in bug #78379, but still don't handle
the case where the properties array is marked as grey first,
which causes a delref to not be performed later.
Fix this by treating the object properties HT the same way as
other refcounted values, including addrefs/delrefs. The object
dtor code already handles properties HT with NULL GC type, so
out of order destruction should not be a problem.
Fixes oss-fuzz #36023.
TMPVAR operands are destroyed using zval_ptr_dtor_nogc(), because
they usually cannot contain cycles. However, there are some rare
exceptions where this is possible, e.g. unserialize() return value.
In such cases we rely on the producing code to root the value. If
a GC run occurs between the rooting and consumption of the value,
we would end up leaking it. To avoid this, root all live TMPVAR
values after a GC run.
Closes GH-7210.
Since PHP 7.4 objects that have a destructor require two GC runs
to be collected. Currently the collection is delayed to the next
automatic GC run. However, in some cases this may result in a large
increase in memory usage, as in one of the cases of bug #79519.
See also bug #78933 and bug #81117 where the current behavior is
unexpected for users.
This patch will automatically rerun GC if destructors were encountered.
I think this should not have much cost, because it is very likely that
objects on which the destructor has been called really are garbage,
so the extra GC run should not be doing wasted work.
Closes GH-5581.
On further consideration, we should be making use of the fact
that zend_object_iterator is also a zend_object here, and let
GC handle the get_gc call on it. Calling get_gc recursively like
this is generally not safe, because there is only one gc_buffer.
This also happens to be much simpler...
Moving the zend_iterator_dtor from dual_it_dtor to dual_it_free_storage
exposed this GC leak in an existing test. Forward the result of the
iterator get_gc to the dual_it get_gc.
We're starting to see a mix between uses of zend_bool and bool.
Replace all usages with the standard bool type everywhere.
Of course, zend_bool is retained as an alias.
While the initial threshold is set to 10001 roots, the threshold
adjustment logic may then set it to 10000. The exact value really
doesn't matter, but we should make it consistent.
We should be doing this anyway to prevent stack overflow, but on
master this is important for an additional reason: The temporary
GC buffer provided for get_gc handlers may get reused if the scan
is performed recursively instead of indirected via the GC stack.
This fixes oss-fuzz #23350.
get_gc() implementations that need to explore heterogeneous data
currently work by computing how many GC entries they need,
allocating a buffer for that and storing it on the object. This
is inefficient and wastes memory, because the buffer is retained
after the GC run.
This commit adds an API for a single global GC buffer, which can
be reused by get_gc implementations (as only one get_gc call is
ever active at the same time). The GC buffer will automatically
grow during the GC run and be discarded at the end.
We no longer protect GC during the destroy phase, so we need to
deal with buffer reallocation.
Note that the implementation of spl_SplObjectStorage_free_storage
will call the destructor of SplObjectStorage, and free the instance properties,
which I think is what caused the root buffer to be reallocated.
(`current` is a pointer for an index within the root buffer?)
This fixes bug #78811 for me.
Closes GH-4935
