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

[WIP, RFC] Problem: lot of use of dynamic memory allocation, leading to performance degradation and non-determism #3911

Draft
wants to merge 58 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 54 commits
Commits
Show all changes
58 commits
Select commit Hold shift + click to select a range
4f84758
Implement a very simple zero-lock message pool to test performance gains
f18m Aug 13, 2019
ea0dc06
Merge branch 'master' of https://github.com/f18m/libzmq.git
f18m Aug 13, 2019
a24f2af
Allow to choose message sizes as well
f18m Aug 13, 2019
1bd2ae1
Allow using env variables to do some basic overriding
f18m Aug 13, 2019
252e8d4
fix typo
f18m Aug 13, 2019
4a30795
add TCP kernel socket buffer setting
f18m Aug 13, 2019
577232e
Merge remote-tracking branch 'upstream/master'
f18m Aug 25, 2019
ff8d79f
Merge remote-tracking branch 'upstream/master'
f18m Aug 28, 2019
00e514e
First implementation of global memory pool for ZMQ
f18m Aug 28, 2019
18c52c4
Remove changes related to graph generation
f18m Aug 28, 2019
a720a31
allow testing up to 8k msg sizes
f18m Aug 28, 2019
b9e1f01
correctly deallocate memory pool blocks
f18m Aug 30, 2019
1649701
fix build with no draft API
f18m Aug 30, 2019
0baafa4
never use allocator for VSM
f18m Aug 31, 2019
59cbfac
Merge branch 'master' of https://github.com/zeromq/libzmq into memory…
May 2, 2020
f0a7a7f
Fixes cmake build
May 2, 2020
f682600
Changes to base class with virtuals
May 2, 2020
3a3d877
Makes max message size dynamic
May 2, 2020
b416348
Dynamically grows mempool
May 2, 2020
cfd4c85
Updates dynamic global pool
May 3, 2020
1dd2304
Removes unnecessary class
May 3, 2020
d06f868
Adds new files to makefile
May 4, 2020
cfa228b
Adds concurrentqueue to sources
May 4, 2020
d96d616
Fixes some warnings
May 4, 2020
caf7798
Adds includes
May 4, 2020
5fbc4cc
Makes initial number of messages a bit more dynamic
May 5, 2020
d2c53c5
Hides global allocator implementation and option when C++11 not avail…
May 5, 2020
348865f
Fixes msvc __cplusplus reporting
May 5, 2020
59c6a6c
Improves <C++11 support
May 5, 2020
1f5abc1
Fixes missing declaration
May 5, 2020
2c29abc
Adds more c++11 guards
May 5, 2020
d7f9452
Adds test and moves queue to external
May 12, 2020
1450beb
Merge branch 'master' of https://github.com/zeromq/libzmq into memory…
May 12, 2020
f4973dc
Fixes bad path
May 12, 2020
3b9ec2f
Fixes bad path
May 12, 2020
a260668
Fixes formatting
May 12, 2020
7dafdf7
Adds destroy to test
May 13, 2020
a3bfc67
Fixes wrong increment
May 13, 2020
cd90418
Fixes bad c+11
May 13, 2020
a575335
Adds a basic concurrent queue
May 13, 2020
74dd371
Removes some debug code
May 13, 2020
e9c3a01
Adds newline and nullptr
May 13, 2020
aaa10dd
Adds a start on function pointer interface
May 13, 2020
77293ab
Adds missing free
May 13, 2020
1309316
Cleans up some includes
May 16, 2020
d666af8
Fixes bad options
May 16, 2020
73807f8
Moves to draft
May 16, 2020
ffcede1
Fixes formatting
May 16, 2020
312f8c3
Updates copyright years
May 16, 2020
bf495f8
Switches to new/delete
May 16, 2020
61870a4
Switches to alternative log2
May 16, 2020
c13f837
More copyright years
May 16, 2020
ea9c5dc
Fixes more formatting
May 16, 2020
20f49ec
Fixes more bad years
May 16, 2020
ba05e8f
Fixes some concurrency issues and bugs
May 17, 2020
afb858d
Merge branch 'master' into memorypool
Sep 19, 2020
32d827d
Adds destroy fn
Sep 19, 2020
84b4f8f
Adds destroy
Jan 16, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 9 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -571,6 +571,8 @@ if(MSVC)

zmq_check_cxx_flag_prepend("/analyze")

zmq_check_cxx_flag_prepend("/Zc:__cplusplus") # enables right reporting of __cplusplus, ref. https://devblogs.microsoft.com/cppblog/msvc-now-correctly-reports-__cplusplus/

# C++11/14/17-specific, but maybe possible via conditional defines
zmq_check_cxx_flag_prepend("/wd26440") # Function '...' can be declared 'noexcept'
zmq_check_cxx_flag_prepend("/wd26432") # If you define or delete any default operation in the type '...', define or
Expand Down Expand Up @@ -849,6 +851,8 @@ endif()
set(cxx-sources
precompiled.cpp
address.cpp
allocator_default.cpp
allocator_global_pool.cpp
channel.cpp
client.cpp
clock.cpp
Expand Down Expand Up @@ -949,9 +953,12 @@ set(cxx-sources
zmtp_engine.cpp
# at least for VS, the header files must also be listed
address.hpp
allocator_default.hpp
allocator_global_pool.cpp
array.hpp
atomic_counter.hpp
atomic_ptr.hpp
basic_concurrent_queue.hpp
blob.hpp
channel.hpp
client.hpp
Expand Down Expand Up @@ -1091,6 +1098,8 @@ set(cxx-sources
zap_client.hpp
zmtp_engine.hpp)

list(APPEND sources ${CMAKE_CURRENT_SOURCE_DIR}/external/mpmcqueue/concurrentqueue.h)

if(MINGW)
# Generate the right type when using -m32 or -m64
macro(set_rc_arch rc_target)
Expand Down
6 changes: 6 additions & 0 deletions Makefile.am
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,17 @@ include_HEADERS = \
include/zmq_utils.h

src_libzmq_la_SOURCES = \
external/mpmcqueue/concurrentqueue.h \
src/address.cpp \
src/address.hpp \
src/allocator_default.cpp \
src/allocator_default.hpp \
src/allocator_global_pool.cpp \
src/allocator_global_pool.hpp \
src/array.hpp \
src/atomic_counter.hpp \
src/atomic_ptr.hpp \
src/basic_concurrent_queue.hpp \
src/blob.hpp \
src/channel.cpp \
src/channel.hpp \
Expand Down
3,712 changes: 3,712 additions & 0 deletions external/mpmcqueue/concurrentqueue.h

Large diffs are not rendered by default.

61 changes: 61 additions & 0 deletions external/mpmcqueue/license.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
This license file applies to everything in this repository except that which
is explicitly annotated as being written by other authors, i.e. the Boost
queue (included in the benchmarks for comparison), Intel's TBB library (ditto),
the CDSChecker tool (used for verification), the Relacy model checker (ditto),
and Jeff Preshing's semaphore implementation (used in the blocking queue) which
has a zlib license (embedded in lightweightsempahore.h).

---

Simplified BSD License:

Copyright (c) 2013-2016, Cameron Desrochers.
All rights reserved.

Redistribution and use in source and binary forms, with or without modification,
are permitted provided that the following conditions are met:

- Redistributions of source code must retain the above copyright notice, this list of
conditions and the following disclaimer.
- Redistributions in binary form must reproduce the above copyright notice, this list of
conditions and the following disclaimer in the documentation and/or other materials
provided with the distribution.

THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY
EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL
THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT
OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION)
HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR
TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE,
EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

---

I have also chosen to dual-license under the Boost Software License as an alternative to
the Simplified BSD license above:

Boost Software License - Version 1.0 - August 17th, 2003

Permission is hereby granted, free of charge, to any person or organization
obtaining a copy of the software and accompanying documentation covered by
this license (the "Software") to use, reproduce, display, distribute,
execute, and transmit the Software, and to prepare derivative works of the
Software, and to permit third-parties to whom the Software is furnished to
do so, all subject to the following:

The copyright notices in the Software and this entire statement, including
the above license grant, this restriction and the following disclaimer,
must be included in all copies of the Software, in whole or in part, and
all derivative works of the Software, unless such copies or derivative
works are solely in the form of machine-executable object code generated by
a source language processor.

THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR
IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
FITNESS FOR A PARTICULAR PURPOSE, TITLE AND NON-INFRINGEMENT. IN NO EVENT
SHALL THE COPYRIGHT HOLDERS OR ANYONE DISTRIBUTING THE SOFTWARE BE LIABLE
FOR ANY DAMAGES OR OTHER LIABILITY, WHETHER IN CONTRACT, TORT OR OTHERWISE,
ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
DEALINGS IN THE SOFTWARE.
1 change: 1 addition & 0 deletions external/mpmcqueue/version.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
https://github.com/cameron314/concurrentqueue/commit/38e6a6f0185a98c3aaf2a95aa109ba041221d527
27 changes: 27 additions & 0 deletions include/zmq.h
Original file line number Diff line number Diff line change
Expand Up @@ -698,6 +698,33 @@ ZMQ_EXPORT int zmq_ctx_get_ext (void *context_,
void *optval_,
size_t *optvallen_);

// ZMQ-provided message-pool implementations. */
// default allocator using malloc/free
#define ZMQ_MSG_ALLOCATOR_DEFAULT 0
// using internally a SPSC queue (cannot be used with inproc maybe?) or perhaps an MPMC queue anyway
#define ZMQ_MSG_ALLOCATOR_PER_THREAD_POOL 1
// using internally a MPMC queue, C++11 required
#define ZMQ_MSG_ALLOCATOR_GLOBAL_POOL 2

ZMQ_EXPORT void *zmq_msg_allocator_new (int type_);
ZMQ_EXPORT int zmq_msg_allocator_destroy (void **allocator_);
ZMQ_EXPORT int
zmq_msg_init_allocator (zmq_msg_t *msg_, size_t size_, void *allocator_);

struct zmq_allocator_t
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be exposed? If so, can the type not be used in the API, as in ZMQ_EXPORT zmq_allocator_t *zmq_msg_allocator_new (int type_);?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If I don't expose this one users cannot provide their own allocator with the current api. This is a design decision. I would allow users to provide their own allocators (maybe they want fixed bins with new, fixed bins with errors, an off the shelves allocator etc.). But of course that depends on how much you want to expose to the user. What do you think about this?

{
// Allocate a chunk of memory of size len and return the pointer
void*(*allocate_fn) (void *allocator, size_t len);

// Deallocate the memory chunk pointed to by data_
void(*deallocate_fn) (void *allocator, void *data_);

// Return true if this is an allocator and alive, otherwise false
bool(*check_tag_fn) (void *allocator);
Copy link
Member

@gummif gummif May 17, 2020

Choose a reason for hiding this comment

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

Is this required? In what circumstances will this be useful?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the users wants to provide his/her own allocator instead of using one of the built-ins.

Copy link
Member

Choose a reason for hiding this comment

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

What I mean is specifically the check_tag_fn function. I don't see why it is necessary at all. Is it used for trying to catch programming errors at runtime?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It would allow you to check if a void pointer is a pointer to what you think it is. It was in the previous PR so I left it in. Need to check if it is actually used somewhere.


void *allocator;
};

/* DRAFT Socket methods. */
ZMQ_EXPORT int zmq_join (void *s, const char *group);
ZMQ_EXPORT int zmq_leave (void *s, const char *group);
Expand Down
16 changes: 16 additions & 0 deletions perf/remote_thr.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "platform.hpp"
#include "../include/zmq.h"
#include <stdio.h>
#include <stdlib.h>
Expand Down Expand Up @@ -67,6 +68,11 @@ int main (int argc, char *argv[])
return -1;
}

#if (defined ZMQ_BUILD_DRAFT_API && defined ZMQ_MSG_ALLOCATOR_GLOBAL_POOL)
// EXPERIMENTAL ALLOCATOR FOR MSG_T
void *allocator = zmq_msg_allocator_new (ZMQ_MSG_ALLOCATOR_GLOBAL_POOL);
#endif

s = zmq_socket (ctx, ZMQ_PUSH);
if (!s) {
printf ("error in zmq_socket: %s\n", zmq_strerror (errno));
Expand Down Expand Up @@ -105,7 +111,11 @@ int main (int argc, char *argv[])
}

for (i = 0; i != message_count; i++) {
#if (defined ZMQ_BUILD_DRAFT_API && defined ZMQ_MSG_ALLOCATOR_GLOBAL_POOL)
rc = zmq_msg_init_allocator (&msg, message_size, allocator);
#else
rc = zmq_msg_init_size (&msg, message_size);
#endif
if (rc != 0) {
printf ("error in zmq_msg_init_size: %s\n", zmq_strerror (errno));
return -1;
Expand Down Expand Up @@ -134,5 +144,11 @@ int main (int argc, char *argv[])
return -1;
}

#if (defined ZMQ_BUILD_DRAFT_API && defined ZMQ_MSG_ALLOCATOR_GLOBAL_POOL)
// IMPORTANT: destroy the allocator only after zmq_ctx_term() since otherwise
// some zmq_msg_t may still be "in fly"
zmq_msg_allocator_destroy (&allocator);
#endif

return 0;
}
58 changes: 58 additions & 0 deletions src/allocator_default.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
/*
Copyright (c) 2019-2020 Contributors as noted in the AUTHORS file

This file is part of libzmq, the ZeroMQ core engine in C++.

libzmq is free software; you can redistribute it and/or modify it under
the terms of the GNU Lesser General Public License (LGPL) as published
by the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.

As a special exception, the Contributors give you permission to link
this library with independent modules to produce an executable,
regardless of the license terms of these independent modules, and to
copy and distribute the resulting executable under terms of your choice,
provided that you also meet, for each linked independent module, the
terms and conditions of the license of that module. An independent
module is a module which is not derived from or based on this library.
If you modify this library, you must extend this exception to your
version of the library.

libzmq is distributed in the hope that it will be useful, but WITHOUT
ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
License for more details.

You should have received a copy of the GNU Lesser General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#include "precompiled.hpp"
#include "allocator_default.hpp"

zmq::allocator_default_t::allocator_default_t ()
{
_tag = 0xCAFEEBEB;
}


zmq::allocator_default_t::~allocator_default_t ()
{
// Mark this instance as dead
_tag = 0xdeadbeef;
}

void *zmq::allocator_default_t::allocate (size_t len_)
{
return operator new (len_, std::nothrow);
}

void zmq::allocator_default_t::deallocate (void *data_)
{
operator delete (data_);
}

bool zmq::allocator_default_t::check_tag () const
{
return _tag == 0xCAFEEBEB;
}
71 changes: 71 additions & 0 deletions src/allocator_default.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/*
Copyright (c) 2019-2020 Contributors as noted in the AUTHORS file

This file is part of libzmq, the ZeroMQ core engine in C++.

libzmq is free software; you can redistribute it and/or modify it under
the terms of the GNU Lesser General Public License (LGPL) as published
by the Free Software Foundation; either version 3 of the License, or
(at your option) any later version.

As a special exception, the Contributors give you permission to link
this library with independent modules to produce an executable,
regardless of the license terms of these independent modules, and to
copy and distribute the resulting executable under terms of your choice,
provided that you also meet, for each linked independent module, the
terms and conditions of the license of that module. An independent
module is a module which is not derived from or based on this library.
If you modify this library, you must extend this exception to your
version of the library.

libzmq is distributed in the hope that it will be useful, but WITHOUT
ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public
License for more details.

You should have received a copy of the GNU Lesser General Public License
along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

#ifndef __ZMQ_I_ALLOCATOR_HPP_INCLUDED__
#define __ZMQ_I_ALLOCATOR_HPP_INCLUDED__

namespace zmq
{
class allocator_default_t
{
public:
allocator_default_t ();

~allocator_default_t ();

static void *allocate_fn (void *allocator_, size_t len_)
{
return static_cast<allocator_default_t *> (allocator_)->allocate (len_);
}

static void deallocate_fn (void *allocator_, void *data_)
{
return static_cast<allocator_default_t *> (allocator_)
->deallocate (data_);
}

static bool check_tag_fn (void *allocator_)
{
return static_cast<allocator_default_t *> (allocator_)->check_tag ();
}

// allocate() typically gets called by the consumer thread: the user app thread(s)
void *allocate (size_t len_);

void deallocate (void *data_);

bool check_tag () const;

private:
// Used to check whether the object is a socket.
uint32_t _tag;

Choose a reason for hiding this comment

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

I see _tag is marked as dead in destructor, but the compiler will optimize that out unless _tag is volatile-qualified (which could have performance implications elsewhere).

};
}

#endif