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

Proxy performance fix #3440

Merged
merged 1 commit into from
Mar 8, 2019
Merged

Proxy performance fix #3440

merged 1 commit into from
Mar 8, 2019

Conversation

emtr
Copy link
Contributor

@emtr emtr commented Mar 7, 2019

As suggested by user somdoron on zeromq-dev mailing list, I implemented the zmq_recv with DONT_WAIT and a benchmark for proxy throughput.
The benchmark shows an important increase of forwarding performance, on my system from ~290kpps to >3Mpps.

Moreover the amount of calls to syscall poll is much lower:

Before: 
% time    syscall 
------ -- ------------ 
 81.77    poll 
 18.22    write

After:

% time    syscall 
------ -- ------------ 
 96.88    write 
  3.11    poll

@bluca
Copy link
Member

bluca commented Mar 7, 2019

Please follow the PR template - most importantly, add a separate commit with a relicensing grant: https://github.com/zeromq/libzmq/blob/master/.github/PULL_REQUEST_TEMPLATE.md

#if defined ZMQ_HAVE_WINDOWS
(void) buffer; // unsupported
#else
pthread_setname_np (pthread_self (), buffer);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

setname is not posix, please remove all this thread name changing as it breaks the build on !Linux

if (str_) {
if (len != strnlen (buffer, sizeof (buffer))) {
printf (
"invalid response string length: expected %zu, received %zu", len,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

../perf/proxy_thr.cpp:123:78: error: ISO C++ does not support the ‘z’ gnu_printf length modifier [-Werror=format]

@bluca
Copy link
Member

bluca commented Mar 7, 2019

Please don't add fixup commits, squash it into the original, or it will break bisecting. Then force push


void recv_string_expect_success (void *socket_, const char *str_, int flags_)
{
const unsigned long len = str_ ? strlen (str_) : 0;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emtr breaks the build on windows:

C:\projects\libzmq\perf\proxy_thr.cpp(110): warning C4267: 'initializing' : conversion from 'size_t' to 'const unsigned long', possible loss of data [C:\projects\build_libzmq\proxy_thr.vcxproj]

if (len != strnlen (buffer, sizeof (buffer))) {
printf (
"invalid response string length: expected %lu, received %lu", len,
strnlen (buffer, sizeof (buffer)));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@emtr breaks the build with clang:

perf/proxy_thr.cpp:130:48: error: format '%lu' expects argument of type 'long unsigned int', but argument 3 has type 'size_t {aka unsigned int}' [-Werror=format=]
               strnlen (buffer, sizeof (buffer)));

perf/proxy_thr.cpp Outdated Show resolved Hide resolved
Improve performance of the proxy forwarding batch of message.
Add throughput benchmark for proxy.
Fix valgrind error reported on unitialized vars
RELICENSE: Add emtr grant
@emtr
Copy link
Contributor Author

emtr commented Mar 8, 2019

Fixed issues (broken builds, formatting); squashed commits

@bluca bluca merged commit 2f5c2f4 into zeromq:master Mar 8, 2019
@bluca
Copy link
Member

bluca commented Mar 8, 2019

thanks!

@emtr
Copy link
Contributor Author

emtr commented Mar 8, 2019

Thanks

@emtr emtr deleted the proxy-performance-fix branch March 8, 2019 15:46
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.

None yet

2 participants