726 Commits over 215 Days - 0.14cph!
Merge: from main
Tests: none
Buildfix: updating stale DLL
Tests: built all modes
Update: Fixing obsolete FreeList usage
Tests: none, trivial change
Update: Consolidating Facepunch.System.Tests
- Cleaned up a bunch of bogus using statements
Tests: Ran combined unit tests - they pass.
Merge: from main
Tests: ran around, gathered, shot, built
Merge: from /main/experiment_reduce_gc_server_refresh
Tests: ran new unit tests, checked Server Browser work
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
Merge: from main
Tests: Built all modes
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
Merge: from main
Tests: built all modes, built small house, cut a tree
Update: Replacing new occurances of obsolete Pool API with new
Tests: built all configs
Merge: from main
Also contains updated codegen binaries to conform with new API
Tests: built all modes
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
Merge: from main
Buildfix for camera shake
Buildfix: ifdef out ExplosionScreenBounceFade logic
Tests: built all target modes, all green
Buildfix: removing usused using declarations
Will trip up PC build
Tests: ran PC build locally
Buildfix: unexpose a variable
Leftover that I missed during cleanup, somehow tripped up the build.
Tests: Build in all modes - pass. Ran unit tests - pass
merge from Pool_Remove_FreeDynamic
Tests: ran new CompanionServerTests 10 times, all green
Bugfix: Close socket properly instead of aborting during test teardown
This is slower, but avoids tripping up the companion server that's polling on data and suddenly gets unexpected remote closure, triggering an exception.
Tests: ran the batch of tests 10 times, no failures
Merge: from main
When testing on main, suddenly discovered that 3rd test batch run would produce an error.
Tests: ran CompanionServerTests multiple times, 2 green batches, 1 red (on teardown) - will fix in next CL
Merge: From Main
Tests: Ran CompanionServerTests couple times, all green
Update: Treat Obsolete warnings as visible, potentially errors.
This allows us to clearly see when we have deprecation warnings. We can still clear the console if it's too spammy. Lastly, because they're treated as warnings and not flat-out-errors it allows us to control when we hard-deprecate via flag to [Obsolete].
Tests: marked a funciton as Obsolete - observed warnings instead of errors. Marked that func as obsolete-error - saw errors.
Merge: merge from main
Lets see how much extra work I got left now
Update: AssetStorage now respects compression settings for Save()
Originally it was doing nothing (because it picked a compression format as textureFormat, but then set textureCompression to Uncompressed), but now it'll try to compress on request. We only have 1 place where we attempted to compress the terrain normal map, but I'm disabling that explicitly (as it would lead to fudged normals).
Tests: Baked Heightmap on CraggyIsland - spat out the same formatted texture as before
Bugfix: Discard alpha channel for generated LUT
Originally it was saved as RGB24, but since my deprecation warning fix I re-enabled Alpha channel. This fixes it.
Tests: none, trivial change
Clean: Fixing depr warnings in Third Party/VertexToolsPro
- Adapted uncompressed texture pixel access, TextureImporterFormat -> TextureImporterCompression
- SceneView.onSceneGUIDelegate -> duringSceneGui
Holy guacamoly, it's there's no more deprecation warnings. Now to double check previous compression conversions, as I think I missed alpha channel handling in a couple places.
Tests: none, trivial changes
Clean: Fixing depr warnings in Third Party/PluginMaster
8 to go.
Tests: none, trivial changes
Clean: Fixing depr warnings in Third Party/CinematicEffects
10 to go
Tests: none, trivial changes
