BIND 10 master, updated. 32fcf336abbd3e989ab37108cc896c7f5d6984b2 Changelog for #537
BIND 10 source code commits
bind10-changes at lists.isc.org
Thu Feb 3 18:28:54 UTC 2011
The branch, master has been updated
via 32fcf336abbd3e989ab37108cc896c7f5d6984b2 (commit)
via 94cb95b1d508541201fc064302ba836164d3cbe6 (commit)
via 81ab8dbc34ed3b181170ecaf553266dfdfaf123f (commit)
via 18706a6018483a20fff84781c5d2fced811c7a4f (commit)
via bf944144b1283d93a5138145cbdb576cea7a9fa0 (commit)
via d1971c14bddf6bcdbc031d1e3195b48ed4bffd62 (commit)
via cfc8341a5ff3f2cc7873689b2b787e9ed6dd1b02 (commit)
from 4d62e02fd9d410441b2f94f982e4a3b4b6e85ab4 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 32fcf336abbd3e989ab37108cc896c7f5d6984b2
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Feb 3 19:28:23 2011 +0100
Changelog for #537
commit 94cb95b1d508541201fc064302ba836164d3cbe6
Merge: 4d62e02fd9d410441b2f94f982e4a3b4b6e85ab4 81ab8dbc34ed3b181170ecaf553266dfdfaf123f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Feb 3 19:25:34 2011 +0100
Merge remote-tracking branch 'origin/trac537'
commit 81ab8dbc34ed3b181170ecaf553266dfdfaf123f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Feb 3 16:50:09 2011 +0100
[trac537] Indentation
commit 18706a6018483a20fff84781c5d2fced811c7a4f
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Thu Feb 3 16:47:22 2011 +0100
[trac537] Typo
commit bf944144b1283d93a5138145cbdb576cea7a9fa0
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Feb 1 12:39:23 2011 +0100
[trac537] Some explaining comments
commit d1971c14bddf6bcdbc031d1e3195b48ed4bffd62
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Feb 1 12:16:30 2011 +0100
[trac537] Use auto_ptr instead of shared_ptr
In some cases, these variables will live only inside one instance and
will never be copied/shared. Therefore using shared_ptr is unnecessary
overhead (allocation of the count object, etc).
commit cfc8341a5ff3f2cc7873689b2b787e9ed6dd1b02
Author: Michal 'vorner' Vaner <michal.vaner at nic.cz>
Date: Tue Feb 1 12:07:53 2011 +0100
[trac537] Put UDPServer variables to nested class
This is to lover the overhead of copying UDPServer - UDPServer is a
coroutine object and it is copied a lot. This way only single shared
pointer is copied instead of 10 of them.
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 6 +
src/lib/asiolink/internal/udpdns.h | 73 ++----------
src/lib/asiolink/udpdns.cc | 221 +++++++++++++++++++++++++++---------
3 files changed, 185 insertions(+), 115 deletions(-)
-----------------------------------------------------------------------
diff --git a/ChangeLog b/ChangeLog
index f0bb4c9..fd03ada 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,9 @@
+ 163. [func] vorner
+ The pimpl design pattern is used in IOServer, with a shared pointer. This
+ makes it smaller to copy (which is done a lot as a sideeffect of being
+ coroutine) and speeds the server up by around 10%.
+ (Trac #537, git 94cb95b1d508541201fc064302ba836164d3cbe6)
+
162. [func] stephen
Added C++ logging, allowing logging at different severities.
Code specifies the message to be logged via a symbol, and the
diff --git a/src/lib/asiolink/internal/udpdns.h b/src/lib/asiolink/internal/udpdns.h
index f29d5c8..5b67ca9 100644
--- a/src/lib/asiolink/internal/udpdns.h
+++ b/src/lib/asiolink/internal/udpdns.h
@@ -155,7 +155,7 @@ public:
/// \brief Check if we have an answer
///
/// \return true if we have an answer
- bool hasAnswer() { return (done_); }
+ bool hasAnswer();
/// \brief Returns the coroutine state value
///
@@ -173,66 +173,17 @@ public:
private:
enum { MAX_LENGTH = 4096 };
- // The ASIO service object
- asio::io_service& io_;
-
- // Class member variables which are dynamic, and changes to which
- // need to accessible from both sides of a coroutine fork or from
- // outside of the coroutine (i.e., from an asynchronous I/O call),
- // should be declared here as pointers and allocated in the
- // constructor or in the coroutine. This allows state information
- // to persist when an individual copy of the coroutine falls out
- // scope while waiting for an event, *so long as* there is another
- // object that is referencing the same data. As a side-benefit, using
- // pointers also reduces copy overhead for coroutine objects.
- //
- // Note: Currently these objects are allocated by "new" in the
- // constructor, or in the function operator while processing a query.
- // Repeated allocations from the heap for every incoming query is
- // clearly a performance issue; this must be optimized in the future.
- // The plan is to have a structure pre-allocate several "server state"
- // objects which can be pulled off a free list and placed on an in-use
- // list whenever a query comes in. This will serve the dual purpose
- // of improving performance and guaranteeing that state information
- // will *not* be destroyed when any one instance of the coroutine
- // falls out of scope while waiting for an event.
- //
- // Socket used to for listen for queries. Created in the
- // constructor and stored in a shared_ptr because socket objects
- // are not copyable.
- boost::shared_ptr<asio::ip::udp::socket> socket_;
-
- // The ASIO-enternal endpoint object representing the client
- boost::shared_ptr<asio::ip::udp::endpoint> sender_;
-
- // \c IOMessage and \c Message objects to be passed to the
- // DNS lookup and answer providers
- boost::shared_ptr<asiolink::IOMessage> io_message_;
-
- // The original query as sent by the client
- isc::dns::MessagePtr query_message_;
-
- // The response message we are building
- isc::dns::MessagePtr answer_message_;
-
- // The buffer into which the response is written
- isc::dns::OutputBufferPtr respbuf_;
-
- // The buffer into which the query packet is written
- boost::shared_array<char> data_;
-
- // State information that is entirely internal to a given instance
- // of the coroutine can be declared here.
- size_t bytes_;
- bool done_;
-
- // Callback functions provided by the caller
- const SimpleCallback* checkin_callback_;
- const DNSLookup* lookup_callback_;
- const DNSAnswer* answer_callback_;
-
- boost::shared_ptr<IOEndpoint> peer_;
- boost::shared_ptr<IOSocket> iosock_;
+ /**
+ * \brief Internal state and data.
+ *
+ * We use the pimple design pattern, but not because we need to hide
+ * internal data. This class and whole header is for private use anyway.
+ * It turned out that UDPServer is copied a lot, because it is a coroutine.
+ * This way the overhead of copying is lower, we copy only one shared
+ * pointer instead of about 10 of them.
+ */
+ class Data;
+ boost::shared_ptr<Data> data_;
};
//
diff --git a/src/lib/asiolink/udpdns.cc b/src/lib/asiolink/udpdns.cc
index 5641802..83f7b3c 100644
--- a/src/lib/asiolink/udpdns.cc
+++ b/src/lib/asiolink/udpdns.cc
@@ -23,6 +23,7 @@
#include <asio.hpp>
#include <asio/deadline_timer.hpp>
+#include <memory>
#include <boost/shared_ptr.hpp>
#include <boost/date_time/posix_time/posix_time_types.hpp>
@@ -46,29 +47,127 @@ using namespace std;
using namespace isc::dns;
namespace asiolink {
+
+/*
+ * Some of the member variables here are shared_ptrs and some are
+ * auto_ptrs. There will be one instance of Data for the lifetime
+ * of packet. The variables that are state only for a single packet
+ * use auto_ptr, as it is more lightweight. In the case of shared
+ * configuration (eg. the callbacks, socket), we use shared_ptrs.
+ */
+struct UDPServer::Data {
+ /*
+ * Constructor from parameters passed to UDPServer constructor.
+ * This instance will not be used to retrieve and answer the actual
+ * query, it will only hold parameters until we wait for the
+ * first packet. But we do initialize the socket in here.
+ */
+ Data(io_service& io_service, const ip::address& addr, const uint16_t port,
+ SimpleCallback* checkin, DNSLookup* lookup, DNSAnswer* answer) :
+ io_(io_service), done_(false), checkin_callback_(checkin),
+ lookup_callback_(lookup), answer_callback_(answer)
+ {
+ // We must use different instantiations for v4 and v6;
+ // otherwise ASIO will bind to both
+ udp proto = addr.is_v4() ? udp::v4() : udp::v6();
+ socket_.reset(new udp::socket(io_service, proto));
+ socket_->set_option(socket_base::reuse_address(true));
+ if (addr.is_v6()) {
+ socket_->set_option(asio::ip::v6_only(true));
+ }
+ socket_->bind(udp::endpoint(addr, port));
+ }
+
+ /*
+ * Copy constructor. Default one would probably do, but it is unnecessary
+ * to copy many of the member variables every time we fork to handle
+ * another packet.
+ *
+ * We also allocate data for receiving the packet here.
+ */
+ Data(const Data& other) :
+ io_(other.io_), socket_(other.socket_), done_(false),
+ checkin_callback_(other.checkin_callback_),
+ lookup_callback_(other.lookup_callback_),
+ answer_callback_(other.answer_callback_)
+ {
+ // Instantiate the data buffer and endpoint that will
+ // be used by the asynchronous receive call.
+ data_.reset(new char[MAX_LENGTH]);
+ sender_.reset(new udp::endpoint());
+ }
+
+ // The ASIO service object
+ asio::io_service& io_;
+
+ // Class member variables which are dynamic, and changes to which
+ // need to accessible from both sides of a coroutine fork or from
+ // outside of the coroutine (i.e., from an asynchronous I/O call),
+ // should be declared here as pointers and allocated in the
+ // constructor or in the coroutine. This allows state information
+ // to persist when an individual copy of the coroutine falls out
+ // scope while waiting for an event, *so long as* there is another
+ // object that is referencing the same data. As a side-benefit, using
+ // pointers also reduces copy overhead for coroutine objects.
+ //
+ // Note: Currently these objects are allocated by "new" in the
+ // constructor, or in the function operator while processing a query.
+ // Repeated allocations from the heap for every incoming query is
+ // clearly a performance issue; this must be optimized in the future.
+ // The plan is to have a structure pre-allocate several "Data"
+ // objects which can be pulled off a free list and placed on an in-use
+ // list whenever a query comes in. This will serve the dual purpose
+ // of improving performance and guaranteeing that state information
+ // will *not* be destroyed when any one instance of the coroutine
+ // falls out of scope while waiting for an event.
+ //
+ // Socket used to for listen for queries. Created in the
+ // constructor and stored in a shared_ptr because socket objects
+ // are not copyable.
+ boost::shared_ptr<asio::ip::udp::socket> socket_;
+
+ // The ASIO-internal endpoint object representing the client
+ std::auto_ptr<asio::ip::udp::endpoint> sender_;
+
+ // \c IOMessage and \c Message objects to be passed to the
+ // DNS lookup and answer providers
+ std::auto_ptr<asiolink::IOMessage> io_message_;
+
+ // The original query as sent by the client
+ isc::dns::MessagePtr query_message_;
+
+ // The response message we are building
+ isc::dns::MessagePtr answer_message_;
+
+ // The buffer into which the response is written
+ isc::dns::OutputBufferPtr respbuf_;
+
+ // The buffer into which the query packet is written
+ boost::shared_array<char> data_;
+
+ // State information that is entirely internal to a given instance
+ // of the coroutine can be declared here.
+ size_t bytes_;
+ bool done_;
+
+ // Callback functions provided by the caller
+ const SimpleCallback* checkin_callback_;
+ const DNSLookup* lookup_callback_;
+ const DNSAnswer* answer_callback_;
+
+ std::auto_ptr<IOEndpoint> peer_;
+ std::auto_ptr<IOSocket> iosock_;
+};
+
/// The following functions implement the \c UDPServer class.
///
-/// The constructor
-UDPServer::UDPServer(io_service& io_service,
- const ip::address& addr, const uint16_t port,
- SimpleCallback* checkin,
- DNSLookup* lookup,
- DNSAnswer* answer) :
- io_(io_service), done_(false),
- checkin_callback_(checkin),
- lookup_callback_(lookup),
- answer_callback_(answer)
-{
- // We must use different instantiations for v4 and v6;
- // otherwise ASIO will bind to both
- udp proto = addr.is_v4() ? udp::v4() : udp::v6();
- socket_.reset(new udp::socket(io_service, proto));
- socket_->set_option(socket_base::reuse_address(true));
- if (addr.is_v6()) {
- socket_->set_option(asio::ip::v6_only(true));
- }
- socket_->bind(udp::endpoint(addr, port));
-}
+/// The constructor. It just creates new internal state object
+/// and lets it handle the initialization.
+UDPServer::UDPServer(io_service& io_service, const ip::address& addr,
+ const uint16_t port, SimpleCallback* checkin, DNSLookup* lookup,
+ DNSAnswer* answer) :
+ data_(new Data(io_service, addr, port, checkin, lookup, answer))
+{ }
/// The function operator is implemented with the "stackless coroutine"
/// pattern; see internal/coroutine.h for details.
@@ -80,27 +179,35 @@ UDPServer::operator()(error_code ec, size_t length) {
CORO_REENTER (this) {
do {
- // Instantiate the data buffer and endpoint that will
- // be used by the asynchronous receive call.
- data_.reset(new char[MAX_LENGTH]);
- sender_.reset(new udp::endpoint());
+ /*
+ * This is preparation for receiving a packet. We get a new
+ * state object for the lifetime of the next packet to come.
+ * It allocates the buffers to receive data into.
+ */
+ data_.reset(new Data(*data_));
do {
// Begin an asynchronous receive, then yield.
// When the receive event is posted, the coroutine
// will resume immediately after this point.
- CORO_YIELD socket_->async_receive_from(buffer(data_.get(),
- MAX_LENGTH),
- *sender_, *this);
+ CORO_YIELD data_->socket_->async_receive_from(
+ buffer(data_->data_.get(), MAX_LENGTH), *data_->sender_,
+ *this);
} while (ec || length == 0);
- bytes_ = length;
-
- /// Fork the coroutine by creating a copy of this one and
- /// scheduling it on the ASIO service queue. The parent
- /// will continue listening for DNS packets while the child
- /// processes the one that has just arrived.
- CORO_FORK io_.post(UDPServer(*this));
+ data_->bytes_ = length;
+
+ /*
+ * We fork the coroutine now. One (the child) will keep
+ * the current state and handle the packet, then die and
+ * drop ownership of the state. The other (parent) will just
+ * go into the loop again and replace the current state with
+ * a new one for a new object.
+ *
+ * Actually, both of the coroutines will be a copy of this
+ * one, but that's just internal implementation detail.
+ */
+ CORO_FORK data_->io_.post(UDPServer(*this));
} while (is_parent());
// Create an \c IOMessage object to store the query.
@@ -108,56 +215,57 @@ UDPServer::operator()(error_code ec, size_t length) {
// (XXX: It would be good to write a factory function
// that would quickly generate an IOMessage object without
// all these calls to "new".)
- peer_.reset(new UDPEndpoint(*sender_));
- iosock_.reset(new UDPSocket(*socket_));
- io_message_.reset(new IOMessage(data_.get(), bytes_, *iosock_, *peer_));
+ data_->peer_.reset(new UDPEndpoint(*data_->sender_));
+ data_->iosock_.reset(new UDPSocket(*data_->socket_));
+ data_->io_message_.reset(new IOMessage(data_->data_.get(),
+ data_->bytes_, *data_->iosock_, *data_->peer_));
// Perform any necessary operations prior to processing an incoming
// query (e.g., checking for queued configuration messages).
//
// (XXX: it may be a performance issue to check in for every single
// incoming query; we may wish to throttle this in the future.)
- if (checkin_callback_ != NULL) {
- (*checkin_callback_)(*io_message_);
+ if (data_->checkin_callback_ != NULL) {
+ (*data_->checkin_callback_)(*data_->io_message_);
}
// If we don't have a DNS Lookup provider, there's no point in
// continuing; we exit the coroutine permanently.
- if (lookup_callback_ == NULL) {
+ if (data_->lookup_callback_ == NULL) {
CORO_YIELD return;
}
// Instantiate objects that will be needed by the
// asynchronous DNS lookup and/or by the send call.
- respbuf_.reset(new OutputBuffer(0));
- query_message_.reset(new Message(Message::PARSE));
- answer_message_.reset(new Message(Message::RENDER));
+ data_->respbuf_.reset(new OutputBuffer(0));
+ data_->query_message_.reset(new Message(Message::PARSE));
+ data_->answer_message_.reset(new Message(Message::RENDER));
// Schedule a DNS lookup, and yield. When the lookup is
// finished, the coroutine will resume immediately after
// this point.
- CORO_YIELD io_.post(AsyncLookup<UDPServer>(*this));
+ CORO_YIELD data_->io_.post(AsyncLookup<UDPServer>(*this));
dlog("[XX] got an answer");
// The 'done_' flag indicates whether we have an answer
// to send back. If not, exit the coroutine permanently.
- if (!done_) {
+ if (!data_->done_) {
CORO_YIELD return;
}
// Call the DNS answer provider to render the answer into
// wire format
- (*answer_callback_)(*io_message_, query_message_,
- answer_message_, respbuf_);
+ (*data_->answer_callback_)(*data_->io_message_, data_->query_message_,
+ data_->answer_message_, data_->respbuf_);
// Begin an asynchronous send, and then yield. When the
// send completes, we will resume immediately after this point
// (though we have nothing further to do, so the coroutine
// will simply exit at that time).
- CORO_YIELD socket_->async_send_to(buffer(respbuf_->getData(),
- respbuf_->getLength()),
- *sender_, *this);
+ CORO_YIELD data_->socket_->async_send_to(
+ buffer(data_->respbuf_->getData(), data_->respbuf_->getLength()),
+ *data_->sender_, *this);
}
}
@@ -165,8 +273,8 @@ UDPServer::operator()(error_code ec, size_t length) {
/// AsyncLookup<UDPServer> handler.)
void
UDPServer::asyncLookup() {
- (*lookup_callback_)(*io_message_, query_message_, answer_message_,
- respbuf_, this);
+ (*data_->lookup_callback_)(*data_->io_message_,
+ data_->query_message_, data_->answer_message_, data_->respbuf_, this);
}
/// Post this coroutine on the ASIO service queue so that it will
@@ -174,8 +282,13 @@ UDPServer::asyncLookup() {
/// whether there is an answer to return to the client.
void
UDPServer::resume(const bool done) {
- done_ = done;
- io_.post(*this);
+ data_->done_ = done;
+ data_->io_.post(*this);
+}
+
+bool
+UDPServer::hasAnswer() {
+ return (data_->done_);
}
// Private UDPQuery data (see internal/udpdns.h for reasons)
More information about the bind10-changes
mailing list