GC is not required for RakLib as it doesn't generate any unmanaged cycles.
Cycles in general do exist (e.g. Server <-> ServerSession), but these are
explicitly cleaned up, so GC wouldn't have any useful work to do.
This has been a long-standing issue since at least 2016, and probably longer.
Heavy use of getBlock(At) could cause the cache to blow up and use all available memory.
Recently, it's become clear that unmanaged cache size is also a problem for GC, because
the large number of objects blows up the GC root buffer. At first, this causes more frequent
GC runs; later, the frequency of GC runs drops, but the performance cost of them goes up
substantially because of the sheer number of objects. We can avoid this by trimming the
cache when we detect that it's exceeded limits.
I've implemented this in such a way that failing to update blockCacheSize in new code
won't have lasting impacts, since the cache count will be recalculated during scheduled
cache cleaning anyway.
Closes#152.
No one in their right mind is going to change the defaults for these anyway.
All this crap does is overwhelm users with stuff they don't understand.
Most of this stuff has no business being modified by non-developers anyway.
This PR replicates the mechanism by which PHP's own GC is triggered: using a dynamically adjusted threshold based on the number of roots and the number of destroyed cycles. This approach was chosen to minimize behavioural changes.
This currently only applies to the main thread. Doing this for other threads is a bit more complicated (and in the case of RakLib, possibly not necessary anyway).
By doing this, we can get more accurate performance profiling. Instead of GC happening in random pathways and throwing off GC numbers, we trigger it in a predictable place, where timings can record it.
This change may also produce minor performance improvements in code touching lots of objects (such as `CraftingDataPacket` encoding`), which previously might've triggered multiple GC runs within a single tick. Now that GC runs wait for `MemoryManager`, it can touch as many objects as it wants during a tick without paying a performance penalty.
While working on this change I came across a number of issues that should probably be addressed in the future:
1) Objects like Server, World and Player that can't possibly be GC'd repeatedly end up in the GC root buffer because the refcounts fluctuate frequently. Because of the dependency chains in these objects, they all drag each other into GC, causing an almost guaranteed parasitic performance cost to GC. This is discussed in php/php-src#17131, as the proper solution to this is probably generational GC, or perhaps some way to explicitly mark objects to be ignored by GC.
2) World's `blockCache` blows up the GC root threshold due to poor size management. This leads to infrequent, but extremely expensive GC runs due to the sheer number of objects being scanned. We could avoid a lot of this cost by managing caches like this more effectively.
3) StringToItemParser and many of the pocketmine\data classes which make heavy use of closures are afflicted by thousands of reference cycles. This doesn't present a major performance issue in most cases because the cycles are simple, but this could easily be fixed with some simple refactors.
Unfortunately, these new formatting codes conflict with the Java strikethrough and underline, so we can't support these anymore.
A TextFormat::javaToBedrock() is provided to strip these codes, or (if these formats become supported via different codes) to convert them to Bedrock variants.
Co-authored-by: Dylan T. <dktapps@pmmp.io>
having two different workflows able to trigger releases is a pain for build number continuity.
perhaps longer term we should source the build number a different way, but these workflows needed restructuring anyway.
fixes#6563
Since #6217 was merged, \pocketmine\PATH no longer includes the path of the original phar.
This means that the frame originating from the phar stub would not get its path cleaned up,
leading to it being incorrectly detected as a plugin frame.
We should probably explore better methods of detecting plugin crashes in the future; however
this fix should solve the immediate issue.
this could be used for a bunch of different things aside from double chests
since the DoubleChestInventory no longer references anything specific about chests,
I figured it was time to generalize this.
This reverts commit 1d2b52732e3c475ddc2bab4e45726d22850e3d5c.
I hadn't considered that the likes of plugins and hoppers need to be
able to interact with double chest inventories as well as players.
If we were to move this logic to the Block side, we'd have to expose
APIs on the Chest block to get the correct inventory lazily. I'm not
sure I want to commit to having getInventory() on the block right now,
as we can't guarantee it's available (see problems around Campfire
inventory on the block).
Short term, it'll probably be better to just expose the logic in
block\Chest for deciding which side the inventories should be on.
having this created by the block was unreliable anyway. If items were set into the block's created inventory before setting the block in the world, the campfire contents would get overridden when the block was next run through readStateFromWorld() anyway.
There needs to be a deeper exploration of how to handle blocks with inventories without requiring plugins to interact with tiles. For now, this isn't the worst solution, but it's not the best either.
turns out relying on scheduled updates for this was a bad idea, since it causes a lot of unnecessary code to run every tick, as well as being problematic for campfire, which doesn't have any blockstates to compare against.
