Skip to content

Add kqueue context #22

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.

Sign up for GitHub

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Add kqueue context #22

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
9609861
Finish coding of kqueue_context
pandazxx Apr 6, 2025
ff45cad
Cleaning up
pandazxx Apr 21, 2025
e47bd38
Update include/beman/net/detail/basic_socket_acceptor.hpp
pandazxx Apr 24, 2025
830c966
Add conditional compilation
pandazxx Apr 26, 2025
f88f85e
Fix CI error, replace CheckIncludeFile with CheckIncludeFileCXX
pandazxx Apr 26, 2025
bd2bb46
Update execution library to latest version
pandazxx Apr 26, 2025
9b3bbe2
Fix Clang 15 compile error
pandazxx Apr 26, 2025
2a2bb54
Move static filters to member variables
pandazxx Apr 26, 2025
448da9e
Fix problems from review feedback
pandazxx Apr 26, 2025
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
34 changes: 28 additions & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
# cmake-format: on

cmake_minimum_required(VERSION 3.23)
project(beman_net VERSION 0.0.0 LANGUAGES CXX)
project(
beman_net
VERSION 0.0.0
LANGUAGES CXX)
set(TARGET_NAME net)
set(TARGET_PREFIX beman.${TARGET_NAME})
set(TARGET_LIBRARY beman_${TARGET_NAME})
Expand All @@ -13,13 +16,32 @@ set(TARGETS_EXPORT_NAME ${CMAKE_PROJECT_NAME})

set(CMAKE_CXX_STANDARD 23)

include(CheckIncludeFileCXX)
include(CheckCXXSourceCompiles)
check_include_file_cxx(sys/event.h NET_HAS_KQUEUE)
if(NET_HAS_KQUEUE)
add_compile_definitions(NET_HAS_KQUEUE)
check_cxx_source_compiles(
"
#include <sys/event.h>
#include <stdint.h>
Copy link
Member

@dietmarkuehl dietmarkuehl May 5, 2025

Choose a reason for hiding this comment

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

Using nullptr the test is strictly C++: I’d use <cstdint> and ::std::-qualify the respective names.

int main() {
struct kevent ev{::uintptr_t(), ::int16_t(), EV_DELETE, ::uint32_t(), ::intptr_t(), nullptr, {}};
Copy link
Member

@dietmarkuehl dietmarkuehl May 5, 2025

Choose a reason for hiding this comment

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

In C++ the struct keyword isn’t needed.

(void)ev;
Copy link
Member

@dietmarkuehl dietmarkuehl May 5, 2025

Choose a reason for hiding this comment

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

I wonder if that should use [[maybe_unused]] kevent ev{ … }; instead. The cast to (void) does work but nothing in the standard says that it needs to suppress an unused variable warning. Also, C style casts are somewhat frowned upon.

}
"
NET_KEVENT_HAS_EXT)
if(NET_KEVENT_HAS_EXT)
add_compile_definitions(NET_KEVENT_HAS_EXT)
endif()
endif()

include(FetchContent)
FetchContent_Declare(
execution
# for local development, use SOURCE_DIR <path-to>/execution
GIT_REPOSITORY https://github.com/bemanproject/execution
GIT_TAG e9c3032
)
execution
# for local development, use SOURCE_DIR <path-to>/execution
GIT_REPOSITORY https://github.com/bemanproject/execution
GIT_TAG 9cd2e66ba312c4396ac222868cc0486891cfa30b)
FetchContent_MakeAvailable(execution)

include(CTest)
Expand Down
1 change: 1 addition & 0 deletions include/beman/net/detail/basic_socket_acceptor.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#ifndef INCLUDED_BEMAN_NET_DETAIL_BASIC_SOCKET_ACCEPTOR
#define INCLUDED_BEMAN_NET_DETAIL_BASIC_SOCKET_ACCEPTOR

#include <beman/net/detail/socket_base.hpp>
#include <beman/net/detail/io_context.hpp>
#include <beman/net/detail/socket_category.hpp>
#include <system_error>
Expand Down
22 changes: 21 additions & 1 deletion include/beman/net/detail/container.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,11 @@
#ifndef INCLUDED_BEMAN_NET_DETAIL_CONTAINER
#define INCLUDED_BEMAN_NET_DETAIL_CONTAINER

#include <algorithm>
#include <beman/net/detail/netfwd.hpp>
#include <cstddef>
#include <iterator>
#include <optional>
#include <variant>
#include <vector>

Expand All @@ -28,6 +31,10 @@ class beman::net::detail::container {
auto insert(Record r) -> ::beman::net::detail::socket_id;
auto erase(::beman::net::detail::socket_id id) -> void;
auto operator[](::beman::net::detail::socket_id id) -> Record&;
// using iterator = decltype(records)::iterator;
// auto begin() -> iterator;
// auto end() -> iterator;
Comment on lines +34 to +36
Copy link
Member

@dietmarkuehl dietmarkuehl May 5, 2025

Choose a reason for hiding this comment

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

I think these declarations should either be included and appropriately defined or removed.

auto find(const Record& r) -> ::std::optional<::beman::net::detail::socket_id>;
};

// ----------------------------------------------------------------------------
Expand Down Expand Up @@ -55,6 +62,19 @@ inline auto beman::net::detail::container<Record>::operator[](::beman::net::deta
return ::std::get<1>(this->records[::std::size_t(id)]);
}

template <typename Record>
inline auto ::beman::net::detail::container<Record>::find(const Record& r)
Copy link
Member

@dietmarkuehl dietmarkuehl May 5, 2025

Choose a reason for hiding this comment

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

This member function can be const.

-> ::std::optional<::beman::net::detail::socket_id> {
auto it = ::std::find_if(records.begin(), records.end(), [&](auto& l) {
return ::std::holds_alternative<Record>(l) && ::std::get<Record>(l) == r;
});

if (it == records.end()) {
return {};
}
return {::beman::net::detail::socket_id(::std::distance(records.begin(), it))};
}

// ----------------------------------------------------------------------------

#endif
#endif
11 changes: 8 additions & 3 deletions include/beman/net/detail/io_context.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,17 @@

// ----------------------------------------------------------------------------

#if defined(NET_HAS_KQUEUE)
Copy link
Member

@dietmarkuehl dietmarkuehl May 5, 2025

Choose a reason for hiding this comment

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

It seems the header guards against being unavailable itself, i.e., this guard isn’t strictly necessary.

#include <beman/net/detail/kqueue_context.hpp>
#endif
#include <beman/net/detail/netfwd.hpp>
#include <beman/net/detail/context_base.hpp>
#include <beman/net/detail/io_context_scheduler.hpp>
#include <beman/net/detail/poll_context.hpp>
#include <beman/net/detail/container.hpp>
#include <cstdint>
#include <sys/socket.h>
#include <unistd.h>
#include <poll.h>
#include <limits>
#include <cerrno>
#include <csignal>

Expand All @@ -29,8 +30,12 @@ class io_context;

class beman::net::io_context {
private:
#if defined(NET_HAS_KQUEUE)
::std::unique_ptr<::beman::net::detail::context_base> d_owned{new ::beman::net::detail::kqueue_context()};
#else
::std::unique_ptr<::beman::net::detail::context_base> d_owned{new ::beman::net::detail::poll_context()};
::beman::net::detail::context_base& d_context{*this->d_owned};
#endif
::beman::net::detail::context_base& d_context{*this->d_owned};

public:
using scheduler_type = ::beman::net::detail::io_context_scheduler;
Expand Down
Loading