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

Upgrade Boost to 1.70, fix inefficient connection handling #6

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")

if(ENABLE_MASON)
# versions in use
set(MASON_BOOST_VERSION "1.65.1")
set(MASON_BOOST_VERSION "1.73.0")
set(MASON_EXPAT_VERSION "2.2.0")
set(MASON_LUA_VERSION "5.2.4")
set(MASON_BZIP2_VERSION "1.0.6")
Expand Down Expand Up @@ -521,7 +521,7 @@ if(ENABLE_MASON)
include_directories(SYSTEM ${CMAKE_CURRENT_SOURCE_DIR}/third_party/libosmium/include)
else()

find_package(Boost 1.54 REQUIRED COMPONENTS ${BOOST_COMPONENTS})
find_package(Boost 1.70 REQUIRED COMPONENTS ${BOOST_COMPONENTS})
add_dependency_includes(${Boost_INCLUDE_DIRS})

find_package(TBB REQUIRED)
Expand Down
17 changes: 10 additions & 7 deletions docker/Dockerfile
Original file line number Diff line number Diff line change
@@ -1,14 +1,15 @@
FROM debian:stretch-slim as builder
FROM debian:buster-slim as builder
ARG DOCKER_TAG
ARG BUILD_CONCURRENCY
RUN mkdir -p /src && mkdir -p /opt
COPY . /src
WORKDIR /src

RUN NPROC=${BUILD_CONCURRENCY:-$(grep -c ^processor /proc/cpuinfo 2>/dev/null || 1)} && \
echo 'deb http://deb.debian.org/debian buster-backports main' > /etc/apt/sources.list.d/backports.list && \
apt-get update && \
apt-get -y --no-install-recommends install cmake make git gcc g++ libbz2-dev libxml2-dev \
libzip-dev libboost1.62-all-dev lua5.2 liblua5.2-dev libtbb-dev -o APT::Install-Suggests=0 -o APT::Install-Recommends=0 && \
libzip-dev libboost1.71-all-dev lua5.2 liblua5.2-dev libtbb-dev -o APT::Install-Suggests=0 -o APT::Install-Recommends=0 && \
echo "Building OSRM ${DOCKER_TAG}" && \
git show --format="%H" | head -n1 > /opt/OSRM_GITSHA && \
echo "Building OSRM gitsha $(cat /opt/OSRM_GITSHA)" && \
Expand All @@ -30,12 +31,14 @@ RUN NPROC=${BUILD_CONCURRENCY:-$(grep -c ^processor /proc/cpuinfo 2>/dev/null ||

# Multistage build to reduce image size - https://docs.docker.com/engine/userguide/eng-image/multistage-build/#use-multi-stage-builds
# Only the content below ends up in the image, this helps remove /src from the image (which is large)
FROM debian:stretch-slim as runstage
FROM debian:buster-slim as runstage
RUN mkdir -p /src && mkdir -p /opt
RUN apt-get update && \
apt-get install -y --no-install-recommends libboost-program-options1.62.0 libboost-regex1.62.0 \
libboost-date-time1.62.0 libboost-chrono1.62.0 libboost-filesystem1.62.0 \
libboost-iostreams1.62.0 libboost-thread1.62.0 expat liblua5.2-0 libtbb2 &&\
RUN echo 'deb http://deb.debian.org/debian buster-backports main' > /etc/apt/sources.list.d/backports.list && \
apt-get update && \
apt-get install -y --no-install-recommends libboost-program-options1.71.0 libboost-regex1.71.0 \
libboost-date-time1.71.0 libboost-chrono1.71.0 libboost-filesystem1.71.0 \
libboost-iostreams1.71.0 libboost-system1.71.0 libboost-thread1.71.0 \
expat liblua5.2-0 libtbb2 &&\
rm -rf /var/lib/apt/lists/*
COPY --from=builder /usr/local /usr/local
COPY --from=builder /opt /opt
Expand Down
4 changes: 2 additions & 2 deletions include/server/connection.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class RequestHandler;
class Connection : public std::enable_shared_from_this<Connection>
{
public:
explicit Connection(boost::asio::io_service &io_service, RequestHandler &handler);
explicit Connection(boost::asio::io_context &io_context, RequestHandler &handler);
Connection(const Connection &) = delete;
Connection &operator=(const Connection &) = delete;

Expand All @@ -60,7 +60,7 @@ class Connection : public std::enable_shared_from_this<Connection>
std::vector<char> compress_buffers(const std::vector<char> &uncompressed_data,
const http::compression_type compression_type);

boost::asio::io_service::strand strand;
boost::asio::strand<boost::asio::io_context::executor_type> strand;
boost::asio::ip::tcp::socket TCP_socket;
boost::asio::deadline_timer timer;
RequestHandler &request_handler;
Expand Down
14 changes: 7 additions & 7 deletions include/server/server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,12 @@ class Server
}

explicit Server(const std::string &address, const int port, const unsigned thread_pool_size)
: thread_pool_size(thread_pool_size), acceptor(io_service),
new_connection(std::make_shared<Connection>(io_service, request_handler))
: thread_pool_size(thread_pool_size), acceptor(io_context),
new_connection(std::make_shared<Connection>(io_context, request_handler))
{
const auto port_string = std::to_string(port);

boost::asio::ip::tcp::resolver resolver(io_service);
boost::asio::ip::tcp::resolver resolver(io_context);
boost::asio::ip::tcp::resolver::query query(address, port_string);
boost::asio::ip::tcp::endpoint endpoint = *resolver.resolve(query);

Expand All @@ -74,7 +74,7 @@ class Server
for (unsigned i = 0; i < thread_pool_size; ++i)
{
std::shared_ptr<std::thread> thread = std::make_shared<std::thread>(
boost::bind(&boost::asio::io_service::run, &io_service));
boost::bind(&boost::asio::io_context::run, &io_context));
threads.push_back(thread);
}
for (auto thread : threads)
Expand All @@ -83,7 +83,7 @@ class Server
}
}

void Stop() { io_service.stop(); }
void Stop() { io_context.stop(); }

void RegisterServiceHandler(std::unique_ptr<ServiceHandlerInterface> service_handler_)
{
Expand All @@ -96,15 +96,15 @@ class Server
if (!e)
{
new_connection->start();
new_connection = std::make_shared<Connection>(io_service, request_handler);
new_connection = std::make_shared<Connection>(io_context, request_handler);
acceptor.async_accept(
new_connection->socket(),
boost::bind(&Server::HandleAccept, this, boost::asio::placeholders::error));
}
}

unsigned thread_pool_size;
boost::asio::io_service io_service;
boost::asio::io_context io_context;
boost::asio::ip::tcp::acceptor acceptor;
std::shared_ptr<Connection> new_connection;
RequestHandler request_handler;
Expand Down
39 changes: 19 additions & 20 deletions src/server/connection.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,9 @@ namespace osrm
namespace server
{

Connection::Connection(boost::asio::io_service &io_service, RequestHandler &handler)
: strand(io_service), TCP_socket(io_service), timer(io_service), request_handler(handler)
Connection::Connection(boost::asio::io_context &io_context, RequestHandler &handler)
: strand(boost::asio::make_strand(io_context)), TCP_socket(strand), timer(strand),
request_handler(handler)
{
}

Expand All @@ -27,12 +28,11 @@ boost::asio::ip::tcp::socket &Connection::socket() { return TCP_socket; }
/// Start the first asynchronous operation for the connection.
void Connection::start()
{
TCP_socket.async_read_some(
boost::asio::buffer(incoming_data_buffer),
strand.wrap(boost::bind(&Connection::handle_read,
this->shared_from_this(),
boost::asio::placeholders::error,
boost::asio::placeholders::bytes_transferred)));
TCP_socket.async_read_some(boost::asio::buffer(incoming_data_buffer),
boost::bind(&Connection::handle_read,
this->shared_from_this(),
boost::asio::placeholders::error,
boost::asio::placeholders::bytes_transferred));

if (keep_alive)
{
Expand Down Expand Up @@ -119,29 +119,28 @@ void Connection::handle_read(const boost::system::error_code &error, std::size_t
// write result to stream
boost::asio::async_write(TCP_socket,
output_buffer,
strand.wrap(boost::bind(&Connection::handle_write,
this->shared_from_this(),
boost::asio::placeholders::error)));
boost::bind(&Connection::handle_write,
this->shared_from_this(),
boost::asio::placeholders::error));
}
else if (result == RequestParser::RequestStatus::invalid)
{ // request is not parseable
current_reply = http::reply::stock_reply(http::reply::bad_request);

boost::asio::async_write(TCP_socket,
current_reply.to_buffers(),
strand.wrap(boost::bind(&Connection::handle_write,
this->shared_from_this(),
boost::asio::placeholders::error)));
boost::bind(&Connection::handle_write,
this->shared_from_this(),
boost::asio::placeholders::error));
}
else
{
// we don't have a result yet, so continue reading
TCP_socket.async_read_some(
boost::asio::buffer(incoming_data_buffer),
strand.wrap(boost::bind(&Connection::handle_read,
this->shared_from_this(),
boost::asio::placeholders::error,
boost::asio::placeholders::bytes_transferred)));
TCP_socket.async_read_some(boost::asio::buffer(incoming_data_buffer),
boost::bind(&Connection::handle_read,
this->shared_from_this(),
boost::asio::placeholders::error,
boost::asio::placeholders::bytes_transferred));
}
}

Expand Down