branchrust_reboot/main/experiment_reduce_gc_server_refreshcancel
23 Commits over 0 Days - ∞cph!
Merge: from main
Tests: ran new unit tests, checked ServerBrowser
Update: StringView gains index and range support
Added tests to cover new functionality. Added more checks for Substring operations
Tests: ran new unit tests, checked ServerBrowser
Update: StringView - prefer ctor overloads instead of default args
Saves a couple branches in some scenarios.
- Also expanded tests to cover all ctors and arg ranges
Tests: Ran new unit tests and checked server browser
Clean: Updating the docs of StringView to compare against Memory
Turns out I missed System.memory during my initial research, which serves a very similar purpose. Sadly, it doesn't cover all our use cases, so we still need StringView - I've mentioned these shortcomings in the xml doc.
Tests: none, trivial change
Update: Finalizing StringView
- StringView's constructor now follow's Span form (source, start, len) instead of (source, start, end). Fixed updated unit tests.
- Reimplemented CompareIgnoreCase via EqualityComparer<T> instead of IEqualityComparer<T> as per MS recommendation.
- Annotated every public method with xml docs
Test: ran unit tests - all green. Checked ServerBrowser - still good.
Update: Expanding HashEquality test for StringView
To confirm that the required interface is implemented correctly and gets invoked.
Tests: Ran the updated unit test
Merge: from main
Tests: built all modes. Server Browser still works
Update: Reimplement tag support in server browser
Fixes the break I left when running the experiment with StringView
Tests: Clicked through server browser's various filter options
Buildfix: Reimplement deleted overload call from last CL
Also reduced number of hashset allocations when tags are empty.
Tests: Went to server browser to check Nexuses
Update: Nuking the allocating StringView.Split
There's a non-allocating overload already present, so going to keep it default to help keep the 0-allocations approach.
Tests: none, trivial changes
Add: unit tests for StringView and it's ignore case comparator
I'll need to merge this Tests assembly with the one in unify_pool_free branch once it's merged (which-ever one goes in first)
Tests: ran them - they pass
Update: Further rounding up StringView
- Added ToString() (makes debugging much nicer too)
- Fixed up wrong exception types
- Added Length property
Tests: ran new unit tests(next cl) for StringView, they pass
Clean: use a cast instead of new expr
Also dead static removal
Tests: none, trivial change
Optim: StringView.Split can use user provided temp storage
ServerInfo uses a pooled list to as a go-between the StringView and HashSet. This provides last noticeable change that reduces an allocation per ctor, with average going down by 1.3 (8.31 -> 7).
Also reinforced empty tags to not allocate a new empty set everytime.
Tests: checked ServerBrowser - tags still recognized.
Optim: Replacing ServerInfo.Region with StringView instead of string
Avoids another optional allocation, avg count went down by 0.6 (8.95 -> 8.31). This looks like mostly it for clear&easy wins.
Tests: Checked server browser, it displayed region tags
Update: Add ServerInfo.Protocol enum
Avoids us creating a string to track which protocol to use. On average, saves about 0.6allocs (9.56 -> 8.95). We already have a similar enum on the game level, but this has the "Default" member.
Tests: none, trivial change
Optim: further reduce allocs for ServerInfo
Couple easy wins - Changeset is unused so removed, construct ConnectionString on demand, avoid identifying tags if already identified.
Should've removed 2allocs per ctor, but data shows reduction by 3allocs on average(12.94 -> 9.56).
Tests: None, trivial changes
Update: Allow StringView to implicitly convert to ReadOnlySpan
This allows StringView to be friendly with APIs that Span, like int.TryParse(). It also removes 3 allocations, and weirdly my tests show a reduction of 4 allocations per ctor invocation on average.
Also merging stragler tag lookup into the main loop(avoids an extra traversal and extra lambda allocation).
Tests: Checked ServerBrowser, player count was sane.
Update: Annmotating ServerCacheQuery to track GC allocs
Should've done it at the start but ah well. This also revealed that Task.Delay doesn't seem to be doing much - worth investigating further.
Tests: took a recording during server list refresh - no errors and could see the samples displayed.
Update: Rewriting ServerInfo to use StringView for Tags
This revealed a bunch of other dependencies that needed to be updated. Some of them got plugged up by string copies, some by uninmplemented StringView.Substring. Overall, seems doable, though revealed a number of shortcomings - for example, we'll need to implement our own <numeric>.TryParse(StringView) logic. On the flipside, it allows us to do String pooling a bit more aggressively in our steam library(if we decide to go that far).
Tests: Opened GameServer, observed list being populated and interactible.
Bugfix: fixing utility algs in StringView
Tests: server browser displayed all results (next CL)
Add: new StringView utility
It's essentially a ReadOnlySpan that keeps alive it's referred-to string, and is able to be stored as a member variable. Only contains a smidgen of algorithms that are needed to use it as part of ServerBrowserList. Goal is to find out how much code we need to update to be able to eliminate most allocations in the browser refresh flow.
Tests: none
