- User Since
- May 31 2022, 6:07 AM (11 w, 3 d)
Tue, Aug 9
Looks good from my end for the most part. A few lingering nit type comments left, but otherwise great.
Sat, Aug 6
Awesome perf win! I play lots of large multiplayer matches, and definitely appreciate seeing the late game speed up with many units.
Jul 4 2022
Usually the overhead from std::function<> comes from one of two sources:
- Heap allocation for the functor/function pointer/callable std::function is wrapping (https://devblogs.microsoft.com/oldnewthing/20200514-00/?p=103749), including CPU cache misses when following the pointer
- Indirection when invoking the type erased std::function through a virtual method call
Jun 13 2022
Curious as well what sort of tooling 0AD has for template scanning/validation? I saw the templateanalyzer under source/tools, but that appears to be about balance analysis, rather than validation.
Is it possible there are other templates with this same issue?
Jun 12 2022
Update from feedback
Jun 1 2022
Hey @wraitii are you planning on cleaning up and landing these changes soon? Asking since I'd like to pull in changes with the AddRecurrentTask API and can either wait for this to land, or put up a diff with an updated version of TimerThread :)
Looking at the details, the ip and port values inferred in the three conditional blocks are not used in the actual connection attempt. But rather those two values are only used to establish a Stun tunnel. For that reason, I'm not convinced that QueryConnectionDetails() would be semantically accurate for the name. Instead, something like SetupStun() would more accurately reflect the intent. Additionally, naively extracting the middle section of TryToConnect() which is gated on g_XmppClient into a second function requires some gymnastics around the return value. There are three possibilities for the return value:
- One value indicating the original code had 'return true' in the initial TryToConnect() implementation, and the new TryToConnect() implementation should have an early 'return true' and not proceed further following the call to 'SetupStun()'
- Another value indicating the original code had 'return false' in the initial TryToConnect() implementation, and the new TryToConnect() implementation should have an early 'return false' and not proceed further following the call to 'SetupStun()'
- Another value indicating the original code extracted into SetupStun() had neither 'return false' nor 'return true' and TryToConnect() should continue to perform 'g_NetClient->SetupConnection(enetClient)' as before
May 31 2022
Added to programming.json credits file
I suppose the underlying assumption here is the queue is expected to be empty the majority of the time, and thus worker threads should be able to idle themselves to free up CPU cores.