Skip to content

add cpu usage report, add multi core support. Add optional undefined …#22

Open
Qubasa wants to merge 2 commits intorbruenig:masterfrom
Qubasa:master
Open

add cpu usage report, add multi core support. Add optional undefined …#22
Qubasa wants to merge 2 commits intorbruenig:masterfrom
Qubasa:master

Conversation

@Qubasa
Copy link
Copy Markdown
Contributor

@Qubasa Qubasa commented Mar 24, 2025

Hey there, this PR is a bit more controversial as it add automatic forking and opens up multiple consecutive ports, and also reports the CPU usage, if you don't wanna merge it like this I understand :-)

@rbruenig
Copy link
Copy Markdown
Owner

Hi, thank you for another PR :) I'll have a proper look on the weekend. Changes seem resonable to on first glance. Only question is regarding the num_cores cmd argument. Do I understand it correctly, that this will spawn n instances that each use/listen on their own port? If that's the case, we might want to rename the option, because it might be misleading for users. It's seems more like a "spawn x instances" option, instead of making a single instance multithreaded

@Qubasa
Copy link
Copy Markdown
Contributor Author

Qubasa commented Mar 31, 2025

@rbruenig Yes that's exactly what it does. Hmm yeah a renaming is a good idea, I changed it now to -i num number of instances to start

@rbruenig
Copy link
Copy Markdown
Owner

rbruenig commented Apr 1, 2025

@Qubasa Thanks, I think that makes it much clearer.

Code otherwise looks good to me, except that the cpu-usage.c doesn't quite match the code formatting of the rest of the repository. If we apply the following cosmetic changes, I will merge the code:

  • file names should use _ instead of - in between words
  • All if / else statements should use parenthesese ({ }), even if the following statement is only a single line
  • Indent size should be 4 spaces

Could you apply these changes :) ?

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants