Skip to content

Commit 0b24686

Browse files
authored
server: minor tweaks to use more cpp features (ggml-org#23785)
* misc(server): add default port to impl RAII * misc(server): register_gcp_compat() can be const * misc(server): use proper cpp const/auto methods * misc(server): do not reset a unique_ptr, use make_unique instead to be exception safe
1 parent a919001 commit 0b24686

2 files changed

Lines changed: 34 additions & 36 deletions

File tree

tools/server/server-http.cpp

Lines changed: 32 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,9 @@
55

66
#include <cpp-httplib/httplib.h>
77

8-
#include <cstdlib>
98
#include <functional>
109
#include <future>
10+
#include <memory>
1111
#include <string>
1212
#include <thread>
1313

@@ -21,7 +21,7 @@ class server_http_context::Impl {
2121
};
2222

2323
server_http_context::server_http_context()
24-
: pimpl(std::make_unique<server_http_context::Impl>())
24+
: pimpl(std::make_unique<Impl>())
2525
{}
2626

2727
server_http_context::~server_http_context() = default;
@@ -62,7 +62,7 @@ struct gcp_params {
6262
}
6363

6464
static std::string getenv(const char * name, const std::string & default_value, bool ensure_leading_slash = false) {
65-
const char * value = std::getenv(name);
65+
const auto * value = std::getenv(name);
6666
if (value == nullptr || value[0] == '\0') {
6767
return default_value;
6868
}
@@ -94,15 +94,15 @@ bool server_http_context::init(const common_params & params) {
9494
auto & srv = pimpl->srv;
9595

9696
#ifdef CPPHTTPLIB_OPENSSL_SUPPORT
97-
if (params.ssl_file_key != "" && params.ssl_file_cert != "") {
97+
if (!params.ssl_file_key.empty() && !params.ssl_file_cert.empty()) {
9898
SRV_INF("running with SSL: key = %s, cert = %s\n", params.ssl_file_key.c_str(), params.ssl_file_cert.c_str());
99-
srv.reset(
100-
new httplib::SSLServer(params.ssl_file_cert.c_str(), params.ssl_file_key.c_str())
99+
srv = std::make_unique<httplib::SSLServer>(
100+
params.ssl_file_cert.c_str(), params.ssl_file_key.c_str()
101101
);
102102
is_ssl = true;
103103
} else {
104104
SRV_INF("%s", "running without SSL\n");
105-
srv.reset(new httplib::Server());
105+
srv = std::make_unique<httplib::Server>();
106106
}
107107
#else
108108
if (params.ssl_file_key != "" && params.ssl_file_cert != "") {
@@ -150,7 +150,7 @@ bool server_http_context::init(const common_params & params) {
150150
// set timeouts and change hostname and port
151151
srv->set_read_timeout (params.timeout_read);
152152
srv->set_write_timeout(params.timeout_write);
153-
srv->set_socket_options([reuse_port = params.reuse_port](socket_t sock) {
153+
srv->set_socket_options([reuse_port = params.reuse_port](const socket_t sock) {
154154
httplib::set_socket_opt(sock, SOL_SOCKET, SO_REUSEADDR, 1);
155155
if (reuse_port) {
156156
#ifdef SO_REUSEPORT
@@ -162,8 +162,8 @@ bool server_http_context::init(const common_params & params) {
162162
});
163163

164164
if (params.api_keys.size() == 1) {
165-
auto key = params.api_keys[0];
166-
std::string substr = key.substr(std::max((int)(key.length() - 4), 0));
165+
const auto key = params.api_keys[0];
166+
const std::string substr = key.substr(std::max(static_cast<int>(key.length() - 4), 0));
167167
SRV_INF("api_keys: ****%s\n", substr.c_str());
168168
} else if (params.api_keys.size() > 1) {
169169
SRV_INF("api_keys: %zu keys loaded\n", params.api_keys.size());
@@ -203,7 +203,7 @@ bool server_http_context::init(const common_params & params) {
203203
}
204204

205205
// remove the "Bearer " prefix if needed
206-
std::string prefix = "Bearer ";
206+
static std::string prefix = "Bearer ";
207207
if (req_api_key.substr(0, prefix.size()) == prefix) {
208208
req_api_key = req_api_key.substr(prefix.size());
209209
}
@@ -232,11 +232,10 @@ bool server_http_context::init(const common_params & params) {
232232
};
233233

234234
auto middleware_server_state = [this](const httplib::Request & req, httplib::Response & res) {
235-
bool ready = is_ready.load();
236-
if (!ready) {
235+
if (!is_ready.load()) {
237236
#if defined(LLAMA_UI_HAS_ASSETS)
238-
auto tmp = string_split<std::string>(req.path, '.');
239-
if (req.path == "/" || (tmp.size() > 0 && tmp.back() == "html")) {
237+
if (const auto tmp = string_split<std::string>(req.path, '.');
238+
req.path == "/" || (!tmp.empty() && tmp.back() == "html")) {
240239
if (const llama_ui_asset * a = llama_ui_find_asset("loading.html")) {
241240
res.status = 503;
242241
res.set_content(reinterpret_cast<const char*>(a->data), a->size, "text/html; charset=utf-8");
@@ -284,17 +283,17 @@ bool server_http_context::init(const common_params & params) {
284283
return httplib::Server::HandlerResponse::Unhandled;
285284
});
286285

287-
int n_threads_http = params.n_threads_http;
286+
auto n_threads_http = params.n_threads_http;
288287
if (n_threads_http < 1) {
289288
// +4 threads for monitoring, health and some threads reserved for MCP and other tasks in the future
290-
n_threads_http = std::max(params.n_parallel + 4, (int32_t) std::thread::hardware_concurrency() - 1);
289+
n_threads_http = std::max(params.n_parallel + 4, static_cast<int32_t>(std::thread::hardware_concurrency() - 1));
291290
}
292291
SRV_INF("using %d threads for HTTP server\n", n_threads_http);
293292
srv->new_task_queue = [n_threads_http] {
294293
// spawn n_threads_http fixed thread (always alive), while allow up to 1024 max possible additional threads
295294
// when n_threads_http is used, server will create new "dynamic" threads that will be destroyed after processing each request
296295
// ref: https://github.com/yhirose/cpp-httplib/pull/2368
297-
size_t max_threads = (size_t)n_threads_http + 1024;
296+
const auto max_threads = static_cast<size_t>(n_threads_http + 1024);
298297
return new httplib::ThreadPool(n_threads_http, max_threads);
299298
};
300299

@@ -310,10 +309,9 @@ bool server_http_context::init(const common_params & params) {
310309
// register static assets routes
311310
if (!params.public_path.empty()) {
312311
// Set the base directory for serving static files
313-
bool is_found = srv->set_mount_point(params.api_prefix + "/", params.public_path);
314-
if (!is_found) {
312+
if (const auto is_found = srv->set_mount_point(params.api_prefix + "/", params.public_path); !is_found) {
315313
SRV_ERR("static assets path not found: %s\n", params.public_path.c_str());
316-
return 1;
314+
return false;
317315
}
318316
} else {
319317
#if defined(LLAMA_UI_HAS_ASSETS)
@@ -353,9 +351,9 @@ bool server_http_context::init(const common_params & params) {
353351
bool server_http_context::start() {
354352
// Bind and listen
355353

356-
auto & srv = pimpl->srv;
357-
bool was_bound = false;
358-
bool is_sock = false;
354+
const auto & srv = pimpl->srv;
355+
auto was_bound = false;
356+
auto is_sock = false;
359357
if (string_ends_with(std::string(hostname), ".sock")) {
360358
is_sock = true;
361359
SRV_INF("%s", "setting address family to AF_UNIX\n");
@@ -367,7 +365,7 @@ bool server_http_context::start() {
367365
SRV_INF("%s", "binding port with default address family\n");
368366
// bind HTTP listen port
369367
if (port == 0) {
370-
int bound_port = srv->bind_to_any_port(hostname);
368+
const auto bound_port = srv->bind_to_any_port(hostname);
371369
was_bound = (bound_port >= 0);
372370
if (was_bound) {
373371
port = bound_port;
@@ -383,7 +381,7 @@ bool server_http_context::start() {
383381
}
384382

385383
// run the HTTP server in a thread
386-
thread = std::thread([this]() { pimpl->srv->listen_after_bind(); });
384+
thread = std::thread([this] { pimpl->srv->listen_after_bind(); });
387385
srv->wait_until_ready();
388386

389387
listening_address = is_sock ? string_format("unix://%s", hostname.c_str())
@@ -440,13 +438,13 @@ static void process_handler_response(server_http_req_ptr && request, server_http
440438
if (response->is_stream()) {
441439
res.status = response->status;
442440
set_headers(res, response->headers);
443-
std::string content_type = response->content_type;
441+
const std::string content_type = response->content_type;
444442
// convert to shared_ptr as both chunked_content_provider() and on_complete() need to use it
445-
std::shared_ptr<server_http_req> q_ptr = std::move(request);
446-
std::shared_ptr<server_http_res> r_ptr = std::move(response);
447-
const auto chunked_content_provider = [response = r_ptr](size_t, httplib::DataSink & sink) -> bool {
443+
std::shared_ptr q_ptr = std::move(request);
444+
std::shared_ptr r_ptr = std::move(response);
445+
const auto chunked_content_provider = [response = r_ptr](size_t, const httplib::DataSink & sink) -> bool {
448446
std::string chunk;
449-
bool has_next = response->next(chunk);
447+
const bool has_next = response->next(chunk);
450448
if (!chunk.empty()) {
451449
if (!sink.write(chunk.data(), chunk.size())) {
452450
return false;
@@ -557,7 +555,7 @@ static std::string path_to_gcp_format(const std::string & path) {
557555
if (c == '/' || c == '-' || c == '_') {
558556
cap = true;
559557
} else {
560-
result += cap ? (char)std::toupper(c) : (char)c;
558+
result += static_cast<char>(cap ? std::toupper(c) : c);
561559
cap = false;
562560
}
563561
}
@@ -581,7 +579,7 @@ static json parse_gcp_predict_response(const server_http_res_ptr & res) {
581579
}
582580
}
583581

584-
void server_http_context::register_gcp_compat() {
582+
void server_http_context::register_gcp_compat() const {
585583
const gcp_params gcp;
586584

587585
if (!gcp.enabled) {
@@ -602,7 +600,7 @@ void server_http_context::register_gcp_compat() {
602600
}
603601

604602
if (!gcp.path_health.empty()) {
605-
auto health_handler = handlers.find("/health");
603+
const auto health_handler = handlers.find("/health");
606604
GGML_ASSERT(health_handler != handlers.end());
607605
get(gcp.path_health, health_handler->second);
608606
}

tools/server/server-http.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ struct server_http_context {
7373

7474
std::string path_prefix;
7575
std::string hostname;
76-
int port;
76+
int port = 8080;
7777
bool is_ssl = false;
7878

7979
server_http_context();
@@ -88,7 +88,7 @@ struct server_http_context {
8888

8989
// Register the Google Cloud Platform (Vertex AI) compat (AIP_PREDICT_ROUTE env var, or /predict)
9090
// Must be called AFTER all other API routes are registered
91-
void register_gcp_compat();
91+
void register_gcp_compat() const;
9292

9393
// for debugging
9494
std::string listening_address;

0 commit comments

Comments
 (0)