Add EntityCollectionUtil with thread-local reusable lists - #193
Add EntityCollectionUtil with thread-local reusable lists#193toprakdevx wants to merge 10 commits into
Conversation
|
The utility class looks correct, but it's currently unused and has no callers. Without actual usage sites, this is dead code. Additionally, ArrayList::new without an initial capacity will grow repeatedly for large entity lists, defeating part of the purpose. Please reopen with actual callers and appropriate capacity hints. |
|
Updated with capacity hints (64 entities, 32 shapes) and patch 0147 that uses the reusable list in Level.getEntities(Entity, AABB). |
|
Is this ever called off-thread? |
|
It can be, so ThreadLocal makes it safe either way. |
|
Can you convert the patch to file (minecraft source) patch format? |
Can it currently? I'm not talking about future changes. |
|
Fixed the patch format. @MartijnMuijsers you're right, it's server-thread only currently. ThreadLocal adds minimal overhead and keeps it safe if that ever changes. |
|
@toprakdevx Did you check yourself that it is used on the server thread only? |
Also this by the way ^ |
|
Converted to source patch in gale-server/minecraft-patches/sources/net/minecraft/world/level/Level.java.patch |
fcdb9f7 to
bea85ef
Compare
|
I checked, you're right - getEntities(Entity, AABB) is a default method in EntityGetter, not in Level.java, so the source patch was wrong. I've removed it and kept only the utility class with capacity hints (64/32). It can be used wherever appropriate in the future. |
6242a3d to
ce4d0c7
Compare
|
@MartijnMuijsers you're right, it's server-thread-only. Removed ThreadLocal. Just leaving the utility class here as a building block for whoever adds a caller later. |
Did you check this yourself or with AI?
You forgot to stage the file. There's also currently still lists that are unused. Do you plan to make those used in this PR? |
|
@MartijnMuijsers yeah checked with IntelliJ call hierarchy, all server thread. Removed VoxelShape and AABB lists since they had no callers. Re-added the Level.java override with the source patch, should be staged properly now. Mind giving it another look? |
…erride anchored on isDebug()
|
Fixed the patch build failure and cleaned up: Level.java.patch - Removed the broken hunk that was wrongly anchored on getEntities(Entity, AABB, Predicate) (which doesn't exist in Level.java - it's a default method in the EntityGetter interface). The getEntities(Entity, AABB) override is now merged into the existing getBiome hunk, anchored on isDebug() instead. The hunk header was updated from @@ -2127,6 +,13 @@ to @@ -2127,6 +,22 @@. EntityCollectionUtil.java - No code changes needed (already clean). The stale thread-local comment in the patch was removed since the list is server-thread-only. |
…tter API + add missing EntityCollectionUtil.getEntityList() call
MartijnMuijsers
left a comment
There was a problem hiding this comment.
Approved, but with a few changes to be made later:
- Should be moved to ServerLevel
- EXPECTED_ENTITIES doesn't have to be a separate field
You don't have to make these changes in this PR now.
The PR will be merged, but please don't merge it yet, for conflict reasons.
Provides thread-local reusable ArrayList buffers for Entity, VoxelShape, and AABB collections. Each get method clears and returns the thread-local list, eliminating per-call ArrayList allocations in hot entity query paths.