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

support for linux scheduler and timer #64

Open
wants to merge 38 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
4d1845b
Turn off warnings about std::result_of_t usage on latest msvc compiler.
lewissbaker Jan 20, 2018
c549df8
support for linux scheduler and timer
neo5167 Jan 27, 2018
af44da4
restructuring the code to reduce ifdefs
neo5167 Feb 7, 2018
26e6c72
conforming to the white space and indentation rules.
neo5167 Feb 12, 2018
3ecdf4a
Update .travis.yml to refer to clang-7
lewissbaker Feb 16, 2018
377deb8
Try clang-7.0 instead of clang-7 package name
lewissbaker Feb 17, 2018
48e1ef5
Fix clang-7 apt package names again
lewissbaker Feb 24, 2018
7d0f2df
Try setting CLANG_VERSION=7 instead of 7.0 in Travis config
lewissbaker Feb 24, 2018
862e99d
[README] Add C++ highlighting for 'schedule_on' (#67)
modocache Feb 27, 2018
936e6ee
Remove possible warnings with redefinitions
attila0x2A Mar 9, 2018
a19b13b
Merge pull request #71 from think-cell/redef_warnings
lewissbaker Mar 27, 2018
4963e33
Update appveyor.yml to ignore x86 optimised failures
lewissbaker Mar 27, 2018
53d4a06
Merge pull request #76 from lewissbaker/appveyor-tweaks
lewissbaker Mar 27, 2018
a4d2c7b
Correct file_write_operation's friend class
attila0x2A Mar 29, 2018
98d98e2
Merge pull request #77 from think-cell/correct_file_write
lewissbaker Apr 4, 2018
b0e5f76
Fix typo in async_generator state description
attila0x2A Mar 8, 2018
14ad4bd
Fix io_service_fixture_with_threads: create given threads count (#75)
grishavanika Apr 5, 2018
bce2a72
Replace <iosfwd> with <ostream> in doctest.h to try and fix linker er…
lewissbaker Apr 6, 2018
5aa1cd1
[AppVeyor] Reenable VS 2017 Preview build variants
lewissbaker Apr 6, 2018
2492c07
Link against vcruntime[d].lib instead of msvcurt[d].lib
lewissbaker Apr 9, 2018
115aa39
fixed the minor comments from lewis.
neo5167 Apr 11, 2018
e5dc2fc
changes for white space based on clang-format.el
neo5167 Apr 12, 2018
d02cab7
copyright block
neo5167 Apr 16, 2018
b23f640
support for linux scheduler and timer
neo5167 Jan 27, 2018
723c049
restructuring the code to reduce ifdefs
neo5167 Feb 7, 2018
b8db839
conforming to the white space and indentation rules.
neo5167 Feb 12, 2018
cc86c9b
fixed the minor comments from lewis.
neo5167 Apr 11, 2018
343fe0a
changes for white space based on clang-format.el
neo5167 Apr 12, 2018
2565005
copyright block
neo5167 Apr 16, 2018
b006b04
rebasing with upstream master and fixing some windows bugs.
Jun 20, 2018
7fec714
fixing merge conflicts.
Jun 20, 2018
87910b8
removing repeated files from cake.
Jun 20, 2018
8007d4f
indentation fixes.
Jun 20, 2018
ab4b9b7
indentation fixes.
Jun 20, 2018
d38a213
more indentation changes. Moving from cc mode to llvm style.
Jun 20, 2018
1696936
reverting some changes since the functions are no longer needed.
Jun 20, 2018
751232b
windows tests passing.
Jun 21, 2018
f870053
All linux tests passing.
Jun 21, 2018
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: 3 additions & 1 deletion config.cake
Original file line number Diff line number Diff line change
Expand Up @@ -221,7 +221,9 @@ elif cake.system.isLinux():
compiler.addLibrary('c++')
compiler.addLibrary('c')
compiler.addLibrary('pthread')

compiler.addLibrary('rt')
compiler.addLibrary('uuid')

#compiler.addProgramFlag('-Wl,--trace')
#compiler.addProgramFlag('-Wl,-v')

Expand Down
63 changes: 63 additions & 0 deletions include/cppcoro/detail/linux.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
#pragma once
Copy link
Owner

Choose a reason for hiding this comment

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

Can you change this to use #ifdef include guards to be consistent with other headers?

Copy link
Owner

Choose a reason for hiding this comment

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

Can you also add a copyright/license header as per the existing source files?

Please attribute copyright to the appropriate entity.

Copy link
Author

Choose a reason for hiding this comment

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

Will have to circle back on this. Have to speak with our team here to get the appropriate rights entity name.

Copy link
Author

Choose a reason for hiding this comment

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

Done.


#include <mqueue.h>
#include <sys/epoll.h>
#include <sys/timerfd.h>
#include <sys/eventfd.h>
#include <fcntl.h>
#include <uuid/uuid.h>
#include <sys/resource.h>
#include <sys/stat.h>
#include <linux/limits.h>
#include <sys/epoll.h>
#include <unistd.h>

typedef int DWORD;
Copy link
Owner

Choose a reason for hiding this comment

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

I'd like to avoid adding new global-scope typedefs and #defines like this in public headers.
I don't want to introduce any issues or potential conflicts with other libraries that may be used in conjunction with cppcoro.

Copy link
Owner

Choose a reason for hiding this comment

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

It looks like this is only required for timeout values inside cppcoro::io_service::timer_thread_state::run(), so maybe its definition could be localised to that file?

Copy link
Author

Choose a reason for hiding this comment

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

Makes sense. Fixed it.

#define INFINITE (DWORD)-1

namespace cppcoro
{
namespace detail
{
namespace linux
{
enum message_type
{
CALLBACK_TYPE,
RESUME_TYPE
};

struct message
{
enum message_type m_type;
void* m_ptr;
};

struct io_state : linux::message
{
using callback_type = void(io_state* state);
callback_type* m_callback;
};

class message_queue
{
private:
mqd_t m_mqdt;
char m_qname[NAME_MAX];
int m_epollfd;
struct epoll_event m_ev;
message_queue();
public:
message_queue(size_t queue_length);
~message_queue();
bool enqueue_message(void* message, message_type type);
bool dequeue_message(void*& message, message_type& type, bool wait);
};

int create_event_fd();
int create_timer_fd();
int create_epoll_fd();

}
}
}
19 changes: 17 additions & 2 deletions include/cppcoro/io_service.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@
# include <cppcoro/detail/win32.hpp>
#endif

#if CPPCORO_OS_LINUX
#include <cppcoro/detail/linux.hpp>
#endif

#include <optional>
#include <chrono>
#include <cstdint>
Expand Down Expand Up @@ -42,8 +46,12 @@ namespace cppcoro
/// actively processing events.
/// Note that the number of active threads may temporarily go
/// above this number.
io_service(std::uint32_t concurrencyHint);

#if CPPCORO_OS_WINNT
io_service(std::uint32_t concurrencyHint);
#endif
#if CPPCORO_OS_LINUX
io_service(size_t queue_length);
Copy link
Owner

Choose a reason for hiding this comment

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

I'm not a huge fan of having this constructor parameter have different meanings on two different platforms - it makes it more difficult to write cross-platform application code.

We could either add two parameters here, or instead pass in some kind of configuration struct that allows overriding defaults?

#endif
~io_service();

io_service(io_service&& other) = delete;
Expand Down Expand Up @@ -147,6 +155,10 @@ namespace cppcoro

void try_reschedule_overflow_operations() noexcept;

void queue_overflow_operation_to_head(schedule_operation* operation) noexcept;
Copy link
Owner

Choose a reason for hiding this comment

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

I think the names of the queue_overflow_operation_to_head/tail functions are a bit misleading.

They are both actually pushing items onto the head of the queue.
It's just that one of them pushes a single item onto the queue, while the other pushes a list of items onto the queue.

What about naming them queue_overflow_operation and queue_overflow_operation_list?


void queue_overflow_operation_to_tail(schedule_operation* operation) noexcept;

bool try_enter_event_loop() noexcept;
void exit_event_loop() noexcept;

Expand All @@ -169,6 +181,9 @@ namespace cppcoro
detail::win32::safe_handle m_iocpHandle;
#endif

#if CPPCORO_OS_LINUX
detail::linux::message_queue* m_mq;
Copy link
Owner

Choose a reason for hiding this comment

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

The message queue could just be a direct member rather than a pointer to a heap-allocated object.
It's just being allocated in the constructor and deleted in the destructor.

#endif
// Head of a linked-list of schedule operations that are
// ready to run but that failed to be queued to the I/O
// completion port (eg. due to low memory).
Expand Down
10 changes: 9 additions & 1 deletion lib/build.cake
Original file line number Diff line number Diff line change
Expand Up @@ -65,20 +65,28 @@ sources = script.cwd([
'cancellation_source.cpp',
'cancellation_registration.cpp',
'lightweight_manual_reset_event.cpp',
'io_service.cpp',
])

extras = script.cwd([
'build.cake',
'use.cake',
])

if variant.platform == "linux":
detailIncludes.extend(cake.path.join(env.expand('${CPPCORO}'), 'include', 'cppcoro', 'detail', [
'linux.hpp',
]))
sources.extend(script.cwd([
'linux.cpp',
]))

if variant.platform == "windows":
detailIncludes.extend(cake.path.join(env.expand('${CPPCORO}'), 'include', 'cppcoro', 'detail', [
'win32.hpp',
]))
sources.extend(script.cwd([
'win32.cpp',
'io_service.cpp',
'file.cpp',
'readable_file.cpp',
'writable_file.cpp',
Expand Down
Loading