Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

IPC perf improvements by freem #1632

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

VReaperV
Copy link
Contributor

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
Copy link
Member

slipher commented Mar 30, 2025

  • 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
Copy link
Member

illwieckz commented Apr 10, 2025

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
Copy link
Member

slipher commented Apr 10, 2025

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).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants