Minimize allocations and avoid rebuilding weighted list when retrieving potential spawns #732
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.
The current implementation of
EventHooks.getPotentialSpawns
is rather inefficient - it fires the event, then constructs an entirely new weighted random list from the event's results, even if no mods changed the potential spawns. This is not very efficient and leads to a lot of allocation overhead beyond the vanilla code, as shown here:This PR solves the issue by making a few changes:
LevelEvent.PotentialSpawns
now allocates its backing list lazily, rather than in the constructor.EventHooks
now takes advantage of that change to reuse the existing weighted list from vanilla if no mods modify potential spawns.With these changes, the only allocation overhead left is that of allocating the
LevelEvent.PotentialSpawns
object itself. Unfortunately, that still shows up in the allocation profiler, but solving it is not really possible unless we move to pooling hot events like this.