IPC perf improvements by freem

[VReaperV]

A set of patches shared by freem that improve IPC performance. I've only added a commit that fixes a compile error with one of these changes.

[slipher]

• fix implicit cast warnings in a header LGTM
• improve SendMsg performances by avoiding malloc calls I don't believe it. I added a log showing when InternalSendMsg is called with nonzero numHandles and it only happens O(1) times per game. (3x each for cgame startup and sgame startup, it seems.) I don't think the ugly global variable is worth optimizing something that only happens a single-digit number of times per game so I would reject this patch.
• InternalRecvMsg reserves before pushing doesn't seem good, for the same reason that handles are only sent O(1) times per game. Then reserving extra memory for them would likely worsen performance.
• recvmsg: use malloc instead of new, reduces a tiny bit time spent there is wrong because it allocates something with malloc that will be freed with delete. Anyway perf improvement is bogus because that allocation is only done once per program lifetime.
• recvmsg: use memset instead of for-loop Obsolete as the same line is rewritten by subsequent commits of this PR.
• recvmsg: use std::fill instead of memset (seems faster?) + Fix compile error in InternalRecvMsg() LGTM (please squash to avoid non-buildable commits)

[illwieckz]

freem sent me those patches earlier in time, I remember his branch implementing that was written above commits adding some benchmark framework around that code to time it precisely, so I believe the whole is actually measured to be faster, so I don't really mind if some of those optimizations may look to be overkill.

The malloc instead of new freed by delete seems to be something to fix, once this is fixed (maybe by using free() where it is deleted?) this looks good to me.

[slipher]

There are no actual optimizations here. The good commits are cleanups. Others just change stuff that is only called a couple of times per game, and so can't possibly have a material impact on performance (unless through the vagaries of the optimizer producing better code accidentally due to a random change).

[necessarily-equal]

Thanks for opening this. I've just rebased this, fixed the build and opened . I've fixed and squashed the std::fill commit and deleted the commit I advised to delete.

Here's a review:

• "fix implicit cast warnings in a header"

Sure

• "improve SendMsg performances by avoiding malloc calls"

Fine for me, although it only helps in the DLL case. I did kept the commit as well.

• "InternalRecvMsg reserves before pushing"

Seems good to me, since we only emplace_back if the handles are correct this won't double or triple the amount of used memory. I kept the commit Update: I was since convinced by slipher that this commit is actually not a good idea. I wouldn't merge it.

• "recvmsg: use memset instead of for-loop"
• "recvmsg: use std::fill instead of memset (seems faster?)"

The version in the second commit sounds good, I did squash these two commits together and made it compile correctly (it was missing a ;).

• "recvmsg: use malloc instead of new, reduces a tiny bit time spent there"

This commit is bogus (malloc vs free). In modern CPP 20 one would use make_unique_for_overwrite for this purpose, but we don't use C++20. Most importantly the allocation function is only called once anyway. I did drop this commit altogether as Slipher suggested.

• "minor perf patch for IPC Reader"

Change seems reasonable, it's a bit longer but it's claimed to be faster so that's okay IMO. I kept the commit.

• "use final keyword in some places"

Sure, if it doesn't break build I'm fine with it. Kept.

[necessarily-equal]

Update: opened #1871