branchrust_reboot/main/baseplayer_serverupdateparallelcancel
119 Commits over 62 Days - 0.08cph!
Clean: comments, spans and names
Tests: none, trivial changes
Optim: UsePlayerUpdateJobs 2 - merge partially ordered chains of snapshots
- This reduces allocations (since we create less tasks), and should help with parallelization (reducing task scheduling overhead), but requires preprocess cost
Tests: on craggy with UsePlayerUpdateJbos 2, flew out of bounds and respawned(this causes 500+ entity snapshots to be sent) 10 times in a row - no exceptions, disconnects and all entities seemed to be present
Bugfix: fix a missing profile scope in FoliageGrid
Tests: used the profiler in editor, no more errors
Bugfix: UsePlayerUpdateJobs 2 - maintain partial ordering of snapshots when sending to player using tasks
- This fixes rare parenting desync that can manifest as a weapon attachment world model not hiding (or something much worse)
Need to optimize it by merging tasks, as this can create too many tasks for the server to process
Tests: flew around on Craggy with fast noclip - attachments didn't "duplicate". Shot animals, switched active items. Tested occlusion with 2p and geared guns
Optim: UsePlayerUpdateJobs - limit duplicate snapshot check to per-player only
Snapshot queues can grow up to 1k per player, so this should reduce the lookup overhead a smidge.
Tests: 2p session on Craggy with UsePlayerUpdateJobs 2
Update: gate parallel snapshot queue behind UsePlayerUpdateJobs 2
All other parts have separation of no tasks under UsePlayerUpdateJobs 1 and tasks allowed under 2, but this slipped my net
Tests: used both in local session on Craggy
Update: move OcclusionCache reset to always happen for disabled UsePlayerUpdateJobs
Don't think this fixes anything, but helps avoid invalid emergency shutdown fallout if it happens in the future.
Tests: none, trivial change
Bugfix: StableObjectCache - properly drop references when clearing
- PlayerCache emergency shutdown logic inspects internal buffer, which tripped up. Change also allows GC
- Added a unit test that validates Clear call.
Tests: Ran the new unit test - it was borked before the fix, gud afterwards
Bugfix: invalidate all non-null managed objects
- Previously it allowed for UnityNull objects to be skipped, leading to desync of StableObjectCache
Tests: ran PlayerCacheTests.StressTest - it now passes
Update: ammend PlayerCacheTests.StressTest to trigger cleanup causing desync with StableObjectCache
Tests: ran test, failing as expected
Optim: UsePlayerUpdateJobs 2 - get rid of ExecutionContext copies when aggregating tasks in SendEntitySnapshots and -WithChildren
Tests: none, trivial change
Optim: UsePlayerUpdateJobs 2 - BasePlayer.WaitForTasks no longer allocs
Could be slower to wait like this, but I doubtr we'll have more than 30 tasks at sync points even on high-pop servers, so it doesn't matter
Tests: 2p on Craggy with UsePlayerUpdateJobs 2
Optim: UsePlayerUpdateJobs 2 - reduce garbage generation of BasePlayer.ApplyChangesParallel
Can't eliminate it fully till we get UniTask, but this should make a dent
Tests: couldn't test thoroughly as missing credentials, but it runs(and doesn't send) under UsePlayerUpdateJobs 2. Profiler confirmed less allocs
Optim: UsePlayerUpdateJobs 2 - stop constantly resizing array, only grow it if needed
Tests: 1p on Craggy with UsePlayerUpdateJobs 2
Optim: UsePlayerUpdateJobs 2 - cache continuation actions
Prevents allocs when no batches are sent, while also makes it cheaper to send when we do have batches.
Tests: craggy with UsePlayerUpdateJobs 2 in editor + checked IL
Bugfix: UsePlayerUpdateJobs 1/2 - fix metabolism being slow
- Fixed by using correct delta times - use tick time length instead of deltaTime (as we accumulate deltaTime until we have a whole tick's worth, then step tick's duration)
- Also fixed marking player hostile if weapon drawn for long enough - same fix as above
Noticed same issue with another bit of code, but this one's wrong in vanilla flow as well - so leaving it as is till later.
Tests: compared rate of healing with log.level Combat 2 after using large medkit on UsePlayerUpdateJobs 0 and 2
Clean: extra "can kick" annotation
Tests: none, trivial change
Clean: fix formatting in BasePlayerr-Server.cs after merges
Tests: none, trivial change
Optim: skip running server occlusion between a pair of same players
Saves us a bunch of extra dictionary entries, running extra occlusion jobs
Tests: 2p sessions with UsePlayerUpdateJobs 0/1/2 on Craggy (with reconnects, kills, occlusion culling)
Bugfix: UsePlayerUpdateJobs 2 - skip trying to run occlusion checks between 2 same players
A little slapdash, but it fixes editor-only CLIENT+SERVER bug of sometimes failing to notify player that they're awake. I'll exclude it from all modes next CL.
Tests: loaded into 2k procgen world with UsePlayerUpdateJobs 2 set from the start 3 tmes - all times no weird zombie state
Optim: UsePlayerUpdateJobs 2 - reduce GC allocations around creating tasks
Surprisingly, this was a struggle to test. Suspicious about a potential bug in unrelated part of UsePlayerUpdateJobs 1/2.
Tests: 2p on Craggy and 2k Procgen map with UsePlayerUpdateJobs 2
Update: Unit tests that experiment with reducing allocation overhead of tasks
Got the following reductions in the unit test:
Garbage size: 1.1KB -> 0.3KB
Alloc Calls: 11 -> 5
The remaining ones seem impossible to remove while still using System.Threading.Task.
Tests: ran the new unit tests
Bugfix: UsePlayerUpdateJobs 1/2 - extend the previous bugfix to the after-validate-recache logic
- Also fixed resetting a bunch of per-player state during the recache step
Tests: loaded map, hover-swim doesn't reproduce anymore (this time no exceptions)
Clean: Replace persistent native WaterFactorCache with transient native allocs
This is slower, but it's not the hot path and it simplifies code and avoids polluting intellisense.
Tests: ran unit tests
Bugfix: UsePlayerUpdateJobs 1/2 - use world pos/rots when running water queries
Previously it used local pos/transf, which was incorrect when parenting happened (excavator as example).
Tests: jumped on excavator, validated that the water queries are running in the right location with UsePlayerUpdateJobs 2
Update: UsePlayerUpdateJobs 1/2 - for blocking-water parented players, store empty WaterInfo instead of garbage
- Expanded TestWaterFactorsConsistency unit test to detect this case
Not marking as bugfix as I couldn't cause any bugs by hacking code. Makes BasePlayer.GetWaterFactors consistent with BasePlayer.WaterFactor
Tests: ran unit tests
Merge: from main
Tests: compiles in editor, player cache unit tests
Bugfix: UsePlayerUpdateJobs 1/2 - use correct indices when filtering out disconnected players
- Added a unit test to validate BasePlayer.FilterInvalidPlayers works correctly
Tests: ran new unit test
Bugfix: avoid potential NRE after player kick for terrain violations
- Rewrote logic to avoid extra IsRealNull checks - we now filter indices instead
Tests: clipped in geometry with noclip
Bugfix: player-like entities that don't support occlusion replicate correctly with UsePlayerUpdateJobs 2
Old way of filtering them allowed them to use cache, which never contained them, so they were culled.
Tests: 2p session on Craggy with UsePlayerrUpdateJobs 2 - spawned NPC and a bot, it didn't despawn
Merge: from main
Tests: none
Bugfix: IndexOutOfRange exception when updating player state cache
Affected both UsePlayerUpdateJobs 1 and 2
Tests: none, simple change
Buildfix: unused variable warning-as-error
Weird it didn't get detected outisde of my branch, but ah well
Tests: built all modes in editor
Bugfix: reset OcclusionCanUseFrameCache on emergency disable of UsePlayerUpdateJobs
Tests: none, trivial change
Clean: extract WaitForTasks
Tests: editor compiles
Update: review fixes #1
- fix formatting in BasePlayer-Mission
- Apply same mission fix from vanilla server flow
- Comments around EventRecord.New and AzureAnalytics.OnPlayerTick warning to avoid scripting API to keep it thread save
Got 2 more changes to make, then think it's ready
Tests: compiles in editor
Update: make batching constants servervars
- New vars are Server.SnapshotTaskBatchCount and Server.DestroyTaskBatchCount
- factored out SendEntityDestroyMessages
Tests: none, trivial changes
Bugfix: recycle filter buffers when there's no results
UsePlayerUpdateJobs 2 only issue
Tests: none, trivial change
Update: ran Generate Code
Tests: none, trivial change
Update: Network++
UsePlayerUpdateJobs 2 can send packets out of order, which needs client-side change (see CL 128169)
Tests: none, trivial change
