[perf] Reduce logger hot-path overhead and sync IO pressure#3210
[perf] Reduce logger hot-path overhead and sync IO pressure#3210Xekep wants to merge 1 commit intoPryaxis:general-develfrom
Conversation
Batch text log flushes, serialize writes under a lock, and avoid expensive caller stack introspection for info/verbose log events. Also remove LINQ First() allocation in SQL failure replay.
|
@greptile review? |
Greptile SummaryThis PR implements targeted performance optimizations for TShock's logging subsystem without changing functionality. Key improvements include:
The changes are well-designed, maintain correctness, and deliver meaningful performance gains for logging hot paths. Confidence Score: 5/5
Important Files Changed
Last reviewed commit: 15e4ce8 |
|
@Xekep do you have any instrumentation that shows the performance gains on these PRs? E.g., before and after profiling, or some indication that these performance gains aren't purely theoretical or, at the very least, within the margin of error for CPU scheduling, etc? |
That matches my real-world experience: before this patch series the server became progressively unresponsive under load (chat and general packet handling lagged noticeably), and after applying the changes responsiveness improved enough to make gameplay stable again. |
You changed like, 9 things at the same time it seems? |
|
This PR probably won’t make a noticeable difference unless:
|
Thanks for the feedback - I agree this won’t matter much on low-log/idle servers. The two changes are cumulative under sustained log volume:
So this is not about needing a 100ms disk or perceiving microseconds directly; it’s about reducing repeated overhead in a hot path at scale. |
|
@Xekep do you know what the juice number * 64 / 128 is? You can find it under valid channels |
😐TShock is pretty sluggish under load right now, so I’d like to focus on fixes. |
|
@Xekep (antiparticles here, stepping in for particles) 😅 You're absolutely right! Let's focus on the performance improvements, rather than worrying about the juice number. How many players are you playing with, and what are the resources allocated to your server? |
The slowdown starts even at low player counts, not only at peak load. Server specs:
|
This method isn’t on a hot path at all, and most of the time in real world it’s called less than once per second. Even at peak load it is still unlikely to be calling it 100 times per second. The |
At the scale of how many players? Realistically I'm worried that you've combined and changed so many things that there are superfluous changes that don't impact much conflating things. The issue here is that your PRs all slightly change performance characteristics, while changing a lot of code in more complex ways. Highly optimized code is typically less understandable because it has to do more acrobatics to do the same thing. TShock is already a highly complex codebase. A big barrier to entry for new contributors is understanding the control flow. We sincerely want to avoid changing the codebase to make it harder to maintain, especially if the performance overhead is negligible at the cost of code maintainability. As you may have noticed, we have not had a steady set of releases. We have, now, had real contributors diving in to solve major problems. Changes like this are difficult to accept without hard numbers. How do we know which PRs you've submitted actually make meaningful improvements and warrant the extra complexity? It is difficult to know without concrete numbers. |
Summary
Scope