435 Commits over 123 Days - 0.15cph!
Merge: from main
Tests: none, no conflicts
Bugfix: Remove assert that forbade double-loading of ItemContainer-owning types
- Removed a separate dead assert
Entities can be synchronized over the network multiple time via doing a whole load, which invalidated my previous logic.
Tests: Spawned a bunch of NPCs and killed them - they didn't generate asserts.
Update: Bringing back Item pooling
- Added a couple extra fields that were missed
Still need to reproduce the NRE we encountered earlier.
Tests: built all modes in editor. Spawned an AK, put an ext-mag on it, threw away to despawn, spawned a new one - didn't have attachments
Merge: from main
Tests: default editor build
Clean: removing obsolete methods from Pool
Tests: built all modes in editor
Merge: from main
Tests: editor build
Merge: from itemcontainer_pooling
Tests: built all modes in editor, booted craggy - no asserts, loaded a save on procgen - no asserts.
Merge: from main
Tests: editor compiled
Update: Assertions to catch leaking ItemContainer on init
- Removed lazy cleanup on init - due to Load-Init flow it's dead code
- Removed a redundant ItemContainer.Clear call
Tests: Booted Craggy - no asserts. Booted a save game from a proc map - no asserts.
Clean: removing redundant ItemContainer.Kill calls
- Removed a comment since it became obsolete
Returning to Pool will "Kill" them.
Tests: None, trivial changes
Clean: removing overriden-but-not-really method from LootableCorpse
Tests: none, trivial change
Clean: simplifying ItemContainer.MarkDirty
Tests: none, trivial changes
Update: ModularVehicleInventory pools ItemContainer
- Had to cause it to discard MVI on server destroy to properly clean up pool containers. Can add MVI pooling later.
Tests: on craggy approached one of spawned vehicles, inserted pistons in the engine then blew up with c4. Pistons dropped, next spawned vehicle didn't have them. Telemetry had expected values.
Update: ContainerIOEntity pools ItemContainer
- reimplemented inventory accessor to be backed by an explicit private var
Tests: spawned small rain collector, destroyed it - saw expected changes in telemetry
Update: ItemBasedFlowRestrictor pools ItemContainer
Tests: in editor on Craggy made fusebox mortal and shot it - saw ItemContainer returned to pool
Update: DroppedItemContainer pools ItemContainer
Tests: spawned a wooden box, added rock and torch, destoyed it with AK. Looted the dropped container, it disappeared. Saw the uptick and downtick in telemetry when expected.
Bugfix: Avoid dangling ItemContainer reference on Clear.
- ItemContainer.Clear now removes items immediately, rather than deferring them to ItemManager's removal queue
- Callbacks are also invoked earlier, before the ItemManager's removal queue pumping
- Instead of sending out per-item ItemContainer-MarkDirty events, we only do 1 for all
- ItemContainer.Kill also clears onPreItemRemove and parent
This prevents a pooling bug where we recycle ItemContainers in same frame before ItemManager removal queue being pumped. It would lead to "invisible" items occupying the inventory.
Tests: Spawned a wooden box, filled it up with stuffs, destroyed it - dropped container spawned with inventory(instead of dropping on the ground). Looting it caused it to despawn(instead of stay in the world).
Bugfix: don't invalidate ItemModContainer's availableSlots
- Also restricted access to availableItems to prevent future accidents and make it easier to reason about list's lifetime.
This is my bug - Pooling resets available slots on ItemContainer, so if they're shared by ref - original source gets nuked.
Tests: got ak47 with ext mag, dropped it, it despawned, spawned new one, tried to put rock into gear slots - it got rejected
Update: output results in micros for Recursion test
Tests: ran the benchmark, confirmed output
Update: PlayerLoot pools Client-side ItemContainers
Tests: tested both server-client looting and just client looting. checked telemetry on both sides - saw expected ups and downs.
Update: forgot to allocate the ItemContainer from pool for IndustrialCrafter
Tests: Setup a T3 workbench with industrial crafter and ak bp(saw an uptick), broke the foundation(saw a downtick), setup again - it was empty as expected
Update: IndustrialCrafter pools ItemContainer
- reimplemented properties as explicit interface props where applicable
- cached LootPanelTitle translated phrase (as it was allocating on every get)
- removed dead code in CanPickup
- got rid of if branch on ServerInit (since we now destroy inventory on entity destroy)
Tests: Setup a T3 workbench with industrial crafter and ak bp(saw an uptick), broke the foundation(saw a downtick), setup again - it was empty as expected
Update: Item & ItemModContainer pools ItemContainer
Tests: spawned an AK, added extended mag, dropped it - saw uptick. after it despawned - got downtick. Spawning another AK gave me a fresh and empty one.
Bugfix: missed original allocation in PlayerCorpse
Tests: none, trivial changes
Update: PlayerCorpse pools ItemContainer
Tests: dressed up, suicide, saw uptick of containers, body disappeared, saw downtick of containers. Repeated, got expected results and didn't see a leak
Update: LootableCorpse pools ItemContainer
Tests: Equipped a horse with backpack and items, killed it - saw the uptick in ItemContainer telemetry, then once corpse despawned, it ticked down.
Update: PlayerInventory pools ItemContianers
Tests: equipped backpack, loaded up on items, killed self and checked that look transferred properly. Confirmed recycling in 2p session by killing the sleeper.
Update: BaseRidableAnimal pools ItemContainers
- Also "fixes" silent id leak by returning it
Test: On Craggy, equipped and shot a horse - saw 2 ItemContainers returned to pool in stats. Spawned another one - they got consumed, and gear was empty.
Clean: remove BaseRidableAnimal.OnInventoryFirstCreated
- Inlined only usecases's event code into init
- Cleaned up double-nested ifdef and a bunch of newlines
It has been used incorrectly anyway and was misleading.
Tests: none, trivial changes
Update: ShopFront pools ItemContainer
- also deleted empty virtual override
Test: 2 player session with a shop front, one was interacting with the front while I destroyed it - items dropped, next front clear
Clean: Remove redundant StorageContainer.ResetState
All the cleanup is already done in DoServerDestroy. Might be worth consolidating these callbacks in the future.
Tests: none, trivial change
Update: StorageContainer now pools ItemContainer
- had to introduce a member variable to avoid funky pool API usage because oif property
- also had to relax empty container assert, as there already are cases that don't clean up their inventory on kill
Tests: on Craggy killed all entities via console, saw the pool container stats go down to 0
Update: ItemContainer supports IPooled
- Minor fix - avoid double returning container UID
- replace a static-readonly with a const
Right now there's only 1 place that pools ItemContainers(reclaim manager), so impact of this is low.
Tests: in softcore killed self and looted reclaim backpack.
Tests:
Optim: avoid copying items on container clear
- Also consolidated it into one function.
Did an overview of existing code, didn't spot cases where we can try to modify the container as we're clearing it.
Tests: killed a bunch of animals, loaded up a wooden storage box then killed it
Update: Adding a couple tail recursion perf tests
- also comes with a vailidity unit test
Going to be used to do a write up for the weekly perf blog post
Tests: Ran these perf tests in editor and player, got kind-of expected results.
Merge: from main
Tests: none
Merge: from fix_hitinfo_pooling
Tests: all modes build in editor, got attacked by animals then injure->kill - correct killer (and no leaks of HitInfo in Pool)
Bugfix: show valid last attacker when suiciding
We used to cache pooled HitInfo which could cause it to be randomly modified by other hit processing. Now we cache a value struct isntead of hitinfo.
Tests: solo session, get attacked by an animal, then run injure -> kill. Used to see self as killer, now see animal
Bugfix: Fix server-side leak of HitInfo for player melee attacks
Still on the lookout for the sourc of bad death screen info
Test: local session, couple hits by p2 and the pool numbers for HitInfo stayed stable
Merge: from main
Tests: none, trivial changes
Merge: from geiger_counter_leak
Brings across the pool.print_memory filter feature.
Tests: none, trivial change
Merge: from main
Tests: built all modes in editor
Clean: remove obsolete logs from GeigerCounter
Tests: none, trivial change
Update: Take marker telemetry every measurement
- Also avoid activating markers during Setup/Shutdown
- Also avoid trying to convert Undefined sampler type to "seconds"
Without these changes perf telemetry was too coarse(1 total value for all measurements * iters)
Tests: Ran it for existing bench tests
Bugfix: Don't record GC data from Warmup runs
Fallout from my first patch
Tests: ran new perf tests with warmup, observed no allocations being recorded(as expected)
Clean: fix 2 Pool obsolete warnings
Tests: none, trivial changes
Merge: from main
- There's 2 new warnings for obsolete pool usage, will fix in next
Tests: Built all modes in editor
Merge: from main
Tests: built all modes in editor
Update: Switch to Dynamic measurements for Pool perf tests
- Also moved the tests into it's own nested class since there'll be more in the future.
Surprised this is not default behavior, but ah well. It does run less measurements, so it avoids random spikes.
Tests: ran the pool perf tests, results line up with previous
Bugfix: Avoid perf degradation with many small Perf Test iterations
Was caused by aggressive GC invocation, which I just ripped out - there's a better alternatives if it's truly needed.
Tests: Ran the Pool bench tests, execution time went down from 3m to 3-5s
