5.24.0 minor release #6598
Merged
5.24.0 minor release #6598
Conversation
This file contains 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
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. <[email protected]>
Automatic merge performed by: https://github.com/pmmp/RestrictedActions/actions/runs/12307996607
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.
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 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.
async workers still use automatic GC for now. We should probably switch to manual GC at some point, but it's not a priority right now.
…ilar to the main thread this avoids costly GC runs during hot code.
This has bitten me on the ass so many times now
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.
Automatic merge performed by: https://github.com/pmmp/RestrictedActions/actions/runs/12440895736
this can happen due to very noisy outputs during conversion, e.g. if there were many unknown blocks.
this reduces the risk of OOM during conversion of large worlds we probably ought to limit the size of region caches for regionized worlds, but that's a problem for another time.
…romise this reduces memory footprint slightly, but more importantly reduces GC workload. Since it also reduces the work done on cache hit, it might *slightly* improve performance, but any improvement is likely to be minimal.
Automatic merge performed by: https://github.com/pmmp/RestrictedActions/actions/runs/12487960388
Automatic merge performed by: https://github.com/pmmp/RestrictedActions/actions/runs/12498201064
Automatic merge performed by: https://github.com/pmmp/RestrictedActions/actions/runs/12510342531
Automatic merge performed by: https://github.com/pmmp/RestrictedActions/actions/runs/12615442245
weak compare isn't needed here since this can be float/float
NBT has better quality type info already
the conventional way is using array_keys and array_fill_keys. Behaviour is more predictable & also avoids benevolent union fuckery from PHPStan.
and workaround PHPStan stupidity
this is a BUT (int|string) under PHPStan, and we don't need the errors. We don't care about this code anyway.
I don't think we get much benefit from this, and the assumption that functions with a return value are pure is sketchy. In any case, it's better to avoid these repeated calls anyway.
Since this doesn't directly correspond to flight speed (it's multiplied by different values depending on whether sprinting or not, and possibly other states), "multiplier" was preferred instead of directly calling it flight speed. Default value is 0.05.
Automatic merge performed by: https://github.com/pmmp/RestrictedActions/actions/runs/12839491049
Automatic merge performed by: https://github.com/pmmp/RestrictedActions/actions/runs/12899221905
…ctable" This reverts commit 97c5902.
![pmmp-admin-bot[bot]](https://avatars.githubusercontent.com/in/1059952?s=60&v=4){kind=small}
ipad54
approved these changes
Jan 22, 2025
|
Draft release 5.24.0 has been created for commit 393d7f7. Please review and publish it. |
dktapps
added
the
Type: Release (!!!)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Prepare 5.24.0 minor release