Avoid repeated CurTime calls in AFK timer

[PrinceOfCookies]

Summary

Small optimization for the AFK timer.

This caches CurTime() once before looping over players, instead of calling it for every player in the loop.

Also checks bots before checking the AFK DarkRPVar.

Changes

Old version:

• called CurTime() for every player
• checked getDarkRPVar("AFK") before IsBot()

New version:

• calls CurTime() once before the loop
• reuses the cached time for every player check
• checks IsBot() before getDarkRPVar("AFK")

Behavior should stay the same.

Benchmark

Tested clientside with a fake player list:

• 128 fake players
• 100,000 timer runs
• timing done with SysTime()

Run	Old	New
1	0.4467s	0.0327s
2	0.4580s	0.0327s
3	0.5357s	0.0431s
4	0.5033s	0.0395s

Old avg	New avg	Improvement
0.4859s	0.0370s	92.4% faster

[FPtje]

Thanks for the contribution! The PR explanation and code look good from a high level view. I'll take a more in depth look when I get back to my laptop.

[PrinceOfCookies]

Sounds good, thanks!

Also just for future reference, if more small optimization changes like this are welcome, would you generally prefer them grouped into one PR, or kept as separate PRs? Obviously with benchmark results/testing included for them.

[FPtje]

To be very honest, I am skeptical about this kind of optimizations. If you walk through the code to find inefficiencies, you will find them, everywhere. There's a whole bunch of optimization techniques, one is indeed calling a function just once, but others are replacing pairs with ipairs, localizing globals at the top of a file, etc.

Benchmarks will indeed show a difference, as yours does above, but those benchmarks do not reflect reality. In this case, the code runs once per second. That means that this optimization saves 0.45 (rounded) seconds every day-ish, assuming constant 128 players. Sure that's 92.4% faster, but nobody is going to notice the difference there.

Over the years I have had quite a few PRs that do exactly this sort of optimization, and I found out that there's a cost to it: risk of (often very subtle!) bugs. One notorious example is that switching pairs for ipairs caused breakages looping over player.GetAll(), because it turns out that in extremely rare cases there are gaps in the table numbering, so players get skipped. This is why Gmod now has a separate player iterator somewhere in the API.

Arguably, the cost of those nasty subtle bugs is higher than the gain of the optimization, and that realization is why I'm skeptical, and also why I want to try my best to test this PR before I merge it.

Now I'm not against any and all optimizations. For me, bottleneck is the key word. Find a place where DarkRP is the thing slowing things down, fix that, and suddenly the optimization has very high value. You can measure this with a profiling tool like e.g. FProfiler (a better tool likely exists by now). Tools like that will tell you which function takes the most time. Both individual calls and accumulated over time.

It's how you find lag spikes, causes of slow servers or clients, etc. Those optimizations bring value, because they solve a noticeable problem.

[FPtje]

Looks like it works. Thanks!